[Toybox] [PATCH] seq: fast path for integers.

Rob Landley rob at landley.net
Sat Dec 26 08:18:52 PST 2020


On 12/15/20 4:05 PM, enh wrote:
> On Sat, Dec 12, 2020 at 3:56 AM Rob Landley <rob at landley.net
> <mailto:rob at landley.net>> wrote:
> 
>     On 12/11/20 1:26 PM, enh via Toybox wrote:
>     > The question "why do we have setlinebuf(stdout)?" came up on the list
>     > recently, and by strange coincidence here it is causing more trouble:
>     > performance rather than usability this time. Just removing the line
>     > buffering alone results in a significant speedup.
> 
>     I have a pending change to NOT line buffer by default, and add a TOYFLAG_LINEBUF
>     to request that in a command's toyflags. I haven't pulled the trigger on it
>     because I don't want to screw up AOSP performance without warning you, but it
>     seems the right thing to do?
> 
>     If your patch is yanking setlinebuf() globally, then my patch seems the right
>     way to do that. :)
> 
> one of these days someone will team up with a bean counter and actually chase
> down where the time is being spent, but the only person i know who ever looked
> at that has moved on to something else, so it's mainly pathological cases
> that get noticed. "why does my build take a minute longer?" type stuff.

That's at _least_ a 3 axis problem. :)

Once I've got current non-systemd Linux From Scratch building under toybox, I
need to dismantle AOSP and turn it into a series of orthogonal stages. The
initial obvious layers are:

1) rebuild prebuilts from source (optional target)
2) build minimal boot to (adb or serial) shell prompt
3) build full text mode userspace using proper init and services.
4) build graphics running a "hello world" app.
5) build full desktop.

But I expect there's a more informed set once I dig into that. I know you don't
use initramfs but hardware bringup guys are probably going to want at least step
2 able to use it...

But that's a post-1.0 todo item and I'm JUST starting to see glimmers of 1.0 at
the end of the tunnel...

>     > +  // Can we avoid %f and just use printf("%lld") for a 10x speedup?
>     > +  incrementl = increment;
>     > +  firstl = first;
>     > +  lastl = last;
>     > +  easy = incrementl==increment && firstl==first && lastl==last && !FLAG(f) &&
>     > +    !TT.precision;
> 
>     How does this differ from just !FLAG(f) && !TT.precision? What do those
>     assignments and comparisons do? (A range check to see if it fits in "int"?
>     Shouldn't 64 bit processors be just as fast with "long"?) Maybe...
> 
> yes, this was a range check to see if it fits in a `long long` (rather than `int`).

Yeah, I reintroduced that as I went along. I made the fast path int because 32
bit systems use libraries for "long long" division so the %10 /10 loop needed
closer examination to see if it was a win. Fast pathing int is a clear win
everywhere _and_ plus or minus 2 billion is probably the common case.

>     So the main bug here is that sprintf("%f") is pathologically slow? Except now
>     with your patch applied it's taking even longer for me? Ah, I see: I did a
>     different change to main() and you were depending on output to a tty defaulting
>     to block buffered when we didn't specify, and mine is doing "line buffer or
>     block buffer" depending on what the command asked for. Sigh. I should buffer
>     into toysh myself...
> 
> (or you should stop trying to work against the grain and just let stdio do its job!)

It keeps changing behavior out from under me with different compiler and libc
versions over the years. (Hence they keep breaking the system's "tee"...)

> the accidental write() instead of xwrite() broke my build, but other than that
> it seems to work. (patch sent.)
> 
> this:
> 
>   if (i<0) {
>     *s++ = '-';
>     i = -i;
>   }
>  
> is wrong for INT_MIN, but we get away with it because we don't take the fast
> path for `./toybox seq -2147483646 -1 -2147483648`.
> 
> that seems to be because of the `if (ii == 2) dd += increment;` line --- because
> although INT_MIN is representable as a double and an int, INT_MIN-1 isn't.
> 
> (patch sent.)

Applied. I think I'm caught up here, but had this reply window open and
half-composed. (Closing tabs...)

Rob



More information about the Toybox mailing list