Re: string-backed FILEs mess

[Rich Felker <dalias@libc.org>]

On Wed, Sep 12, 2018 at 11:43:06AM -0400, Rich Felker wrote:
> > The __string_read approach shouldn't be all that much slower, though. It
> > only adds overhead when the read buffer is empty, which is going to be
> > on the first call and then every 256 characters. And the overhead is
> > mostly a memchr()... on second thought, that might take a while.
> 
> Ah, I forgot that __string_read is still doing the hack of having
> buffer pointers point directly to the string, not copying it, so that
> ungetc is unsafe (in general; the intscan/floatscan/shgetc framework
> has a way to handle it). So it's not a copy, but it is a memchr of N
> to N+(-N&255) bytes, where N is the length of the input item. Even if
> N is short, the input item might lie in a longer string, and if you
> have a sequence of short numbers (worst-case, most single-digit) in a
> long string of numbers, you'll end up repeatedly scanning for null up
> to 255 bytes past the end of the number. Just lowering the scanahead
> from 256 to something like 16-24 should make this largely irrelevant,
> though. Such tuning should probably be specific to caller (strto* vs
> vsscanf, since the latter is likely to be reading multiple fields and
> possibly non-numeric fields).

OK, some measurements. The test is making a string of " 1" repeated
10M times and looping through it with strtol updating end pointer.

With existing strtol, it takes 0.91s.

With __string_read and existing 256-byte unit, it takes 5.76s.

That's a pretty huge slowdown.

Lowering the step from 256 to 24, it takes 2.92s. Better but still
unacceptably slower.

Inlining the buffer setup code and memchr so that __string_read
doesn't have to get called unless it's exhausted (and doesn't get
called in the test) gets it down to 2.00s with a step of 24, and 1.75s
with a step of 8.

OK, I've been properly initializing the FILE rather than leaving it
uninitialized except for the important fields like the old code did.
Changing that, it's 1.44s with step 8, 1.60s with step 24. I also
confirmed that this version of the code is almost as fast as the
existing code with the memchr removed (just assuming it can read
ahead).

I'll see what else I can find but it doesn't look like there's a way
to fix this without either making it a minimum of ~1.5x slower at this
particular worst-case, or redesigning the backend not to use a fake
FILE but some different abstraction.

Rich