rewriting config_*, xmalloc_fget*

[Timo Teräs]

On 06/15/2011 03:14 PM, Rich Felker wrote:
> On Wed, Jun 15, 2011 at 01:49:30PM +0300, Timo Teräs wrote:
>> Hi all,
>>
>> As recently discussed on the 'Significant performance problems with
>> modprobe', the config_* and xmalloc_fget* API has some performance issues:
>> 1. It uses locking getc
>> 2. It does malloc/realloc/free for each line read
>>
>> Now (1) is relatively simply fixable with getdelim(), getline() or
>> getc_unlocked(). And even get_line_from_file.c has comments that fixing
>> of (1) should about double the speed.
> 
> The difference between getc_unlocked and getc should be fairly small.
> If getc_unlocked is a macro, probably at best 2.5-3x, and if it's a
> function, probably at best 1.5-2x. getline/getdelim would make a much
> bigger difference.

What makes you think so?

E.g. uclibc getline and getdelim implementations internally use
__GETC_UNLOCKED macro, which is what you would get with getc_unlocked() too.

Actually getc_unlocked is likely faster, because getdelim/getline still
do regular locking once for each line read. And there just simple isn't
_unlocked variants of them.

We should be really using _unlocked here anyway if available. Since the
parsing api is not intended to be multithreaded anyway, where as regular
stdio is.

>> However, (2) is trickier, because the whole API is designed exactly for
> 
> I'm not convinced that (2) has any bearing whatsoever on performance
> unless you're using some hideous malloc implementation that calls mmap
> for each allocation. Most calls to malloc (when new brk/mmap isn't
> required) should cost about the same as most calls to getc (when
> buffer isn't empty).

This depends very much on your malloc implementation. E.g. on uclibc
calling malloc guarantees that you do at least one mutex lock/unlock
operation. And yes, it will give you performance penalty similar to not
using the _unlocked variants.

If you want to optimise things, let's do it properly.