[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 30 Nov 2018 19:02:29 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: stdio glitch & questions
On Sat, Dec 01, 2018 at 09:15:56AM +1100, Xan Phung wrote:
> Thanks for the quick answer, and I've taken a look at the pre-2011 fwrite.c
> code, and using SYS_writev is indeed much cleaner code!
>
> See below on my proposed answer to your question about what the "cutoff"
> should be for copying. (SYS_writev is fully retained, but the 2nd iovec
> element will very often be zero length in this proposal, which makes
> emulation of SYS_writev much more efficient).
>
> On Sat, 1 Dec 2018 at 03:10, Rich Felker <dalias@...c.org> wrote:
>
> >
> > It would probably be welcome to make __stdio_write make use of
> > SYS_write when it would be expected to be faster (len very small), but
> > I'm not sure what the exact cutoff should be.
> >
> >
> My proposal is the cutoff be 5-8 bytes (on 32 bit CPUs) , and on 64 bit
> CPUs, 9-16 bytes.
>
> The cutoffs are selected in such a way that the "no copy" loop (searching
> for '\n') always ends on a word aligned position (opening the door to
> future optimisations by using word-at-a-time search for '\n' instead of
> byte-at-a-time). The "copy" branch is also guaranteed to only be a double
> word at most, but a minimum of a single word (allowing a two word memcpy to
> be done with just a 2x load/mask/store word code sequence). Some example
> code is shown to give a general idea of word-aligning the cutoff amount
> (but not yet doing word-at-a-time searching of '\n', or optimised two word
> memcpy).
>
> *CURRENT __fwritex.c CODE (lines 12-20)*:
>
>
> if (f->lbf >= 0) {
>
> /* Match /^(.*\n|)/ */
>
> for (i=l; i && s[i-1] != '\n'; i--);
>
> if (i) {
>
> size_t n = f->write(f, s, i);
>
> if (n < i) return n;
>
> s += i;
>
> l -= i;
>
> }
>
> }
>
>
>
> *PROPOSED*:
>
> size_t i, len;
> if (f->lbf >= 0) {
> const unsigned char *t = ALIGN(s+sizeof(size_t)*2);
> for (i = l+s-t; ; i--) {
> if (i <= 0) { /* SHORT LINE - copy up to 16 bytes into f->wpos
> buffer and then flush line */
> for (j = t-s; j && s[j-1] != '\n'; j--);
> if (j) {
> memcpy(f->wpos, s, j); f->wpos += j;
> size_t n = f->write(f, t, 0);
> if (n < 0) return n;
> s += j;
> l -= j;
> } break;
> }
> if (t[i-1] == '\n') {
> size_t n = f->write(f, s, len = i+t-s);
> if (n < len) return n;
> s += len;
> l -= len;
> break;
> }
> }
> }
I've been trying to understand what you're trying to do. It seems you
chose to work at the point of line-buffered flush logic, since that
happens to be the only case where f->write is called with an argument
that might fit in the remaining buffer space.
As written the alignment logic and pointer arithmetic is invalid; the
sums/differences are out of bounds of the array, and i<=0 is not
meaningful since i has an unsigned type (and so does l+s-t). But even
if it could be made correct, it's all completely unnecessary and just
making the code slower and less readable.
If __fwritex were the right place for this code, all you would need to
do is check whether i<16 (or whatever threshold) before calling
f->write, and if so, memcpy'ing it to the buffer then calling f->write
with a length of 0. However, then you could not use the return value
of f->write to determine if it succeeded (see how fflush and fseek
have to deal with this case). Contrary to what your code assumes,
f->write does not (and cannot, since the type is unsigned) return a
negative value on error.
Instead, I think it probably makes more sense to put the logic in
__stdio_write, but this will also be somewhat nontrivial to work in.
At least the "iovcnt == 2 ? ..." logic needs to be adapted to
something like "rem > len ? ...". Before the loop should probably be
something like "if (len < f->wend-f->wpos && len <= 16) ..." to
conditionally copy the new data into the buffer.
Do you see any reason to prefer doing it in __fwritex?
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
