[Toybox] [PATCH] tr: fix pathological flushing.

[Rob Landley]

On 12/4/20 1:58 PM, enh via Toybox wrote:
> The AOSP build doesn't use tr (or anything that's still in pending), but
> the kernel folks have been more aggressive. They found that tr's
> pathological flushing was adding minutes to their build times.

+  while ((n = read(0, toybuf, sizeof(toybuf)))) {
+    if (!FLAG(d) && !FLAG(s)) {
+      for (dst = 0; dst < n; dst++) toybuf[dst] = TT.map[toybuf[dst]];

And when read returns -1 what happens?

Sounds like time for me to cleanup and promote this command. (Let's see, my todo
entry for tr is: "TODO: -t (truncate) -a (ascii)" and I do NOT remember what -a
is but I think I was going to make tr support utf8? Which required a redesign.
Possibly it should be -U to support utf8 instead but everything ELSE supports
utf8 by default, because planet. Hmmm...)

> Just removing the fflush() made tr significantly faster for my trivial
> test, but still slow, with all the time going into stdio.

Single byte writes suck no matter how you slice 'em.

> Rewriting the
> loop to modify toybuf in place and then do one write per read made most
> of the difference, but special-casing the "neither -d nor -s" case made
> a measurable difference too on a Xeon.

Sigh. I have this patch in my tree, which I haven't applied yet because I don't
have the regression test setup to see what it would slow down in the AOSP build:

--- a/main.c
+++ b/main.c
@@ -103,7 +103,7 @@ void toy_singleinit(struct toy_list *which, char *argv[])
       // that choose non-UTF-8 locales. macOS doesn't support C.UTF-8 though.
       if (!setlocale(LC_CTYPE, "C.UTF-8")) setlocale(LC_CTYPE, "");
     }
-    setlinebuf(stdout);
+    setvbuf(stdout, 0, (which->flags & TOYFLAG_LINEBUF) ? _IOLBF : _IONBF, 0);
   }
 }

--- a/lib/toyflags.h
+++ b/lib/toyflags.h
@@ -32,6 +32,9 @@
 // Suppress default --help processing
 #define TOYFLAG_NOHELP   (1<<10)

+// Line buffered stdout
+#define TOYFLAG_LINEBUF  (1<<11)
+
 // Error code to return if argument parsing fails (default 1)
 #define TOYFLAG_ARGFAIL(x) (x<<24)

--- a/toys/posix/grep.c
+++ b/toys/posix/grep.c
@@ -10,9 +10,9 @@
 * echo hello | grep -f </dev/null
 *

-USE_GREP(NEWTOY(grep,
"(line-buffered)(color):;(exclude-dir)*S(exclude)*M(include)*ZzEFHIab(byte-offset)h(no-filename)ino(only-matching)rRsvwcl(files-with-matches)q(quiet)(silent)e*f*C#B#A#m#x[!wx][!EFw]",
TOYFLAG_BIN|TOYFLAG_ARGFAIL(2)))
-USE_EGREP(OLDTOY(egrep, grep, TOYFLAG_BIN|TOYFLAG_ARGFAIL(2)))
-USE_FGREP(OLDTOY(fgrep, grep, TOYFLAG_BIN|TOYFLAG_ARGFAIL(2)))
+USE_GREP(NEWTOY(grep,
"(line-buffered)(color):;(exclude-dir)*S(exclude)*M(include)*ZzEFHIab(byte-offset)h(no-filename)ino(only-matching)rRsvwcl(files-with-matches)q(quiet)(silent)e*f*C#B#A#m#x[!wx][!EFw]",
TOYFLAG_BIN|TOYFLAG_ARGFAIL(2)|TOYFLAG_LINEBUF))
+USE_EGREP(OLDTOY(egrep, grep, TOYFLAG_BIN|TOYFLAG_ARGFAIL(2)|TOYFLAG_LINEBUF))
+USE_FGREP(OLDTOY(fgrep, grep, TOYFLAG_BIN|TOYFLAG_ARGFAIL(2)|TOYFLAG_LINEBUF))

 config GREP
   bool "grep"

I started down that path because changing everything to line buffered by default
is why we had to add an extra flush to passwd() and broke "watch". (The second
is cosmetic breakage, but still...)

I would very much LIKE to move in the direction of knowing what commands
actually need (having line buffered be something we switch on in commands rather
than switch on everywhere by default), but I don't want to surprise you by
pulling the trigger on that one without some sort of coordination.

(Also, line buffering sucks because it'll flush at the buffer size anyway so
you're not guaranteed to get a full line of contiguous output. What it REALLY
wants is nagle's algorithm for stdout but no libc ever bothered to IMPLEMENT it,
possibly because of the runtime expense... Ahem. My point is commands should
probably do sane output blocking on their own.)

Rob