On Wednesday, June 26, 2019 4:04:50 AM EDT you wrote: > What's the motivation for this? Are you running into performance > issues with yes(1), or is this just for fun to increase the throughput > for some benchmark? One of yes(1)'s uses (besides the intended use of making interactive programs work noninteractively) is to generate a high volume stream of data, as /dev/ urandom will only go up around 10MiB/s on most devices. I was doing some testing for a personal program to see how well it could handle an influx of data, and I noticed that the sbase yes is much slower than the GNU and FreeBSD implementations. So, I patched the sbase one quickly and got it to run faster than many implementations (GNU yes being a major exception, running at ~4GiB/s on my machine).
> I'd be in favor of changing yes if it increases readability (while > terse, the existing logic is a bit hard to follow), but I don't think > this patch does that. I agree, and this patch doesn't do that, no. > On 2019-06-25, AGitBoy <[email protected]> wrote: > > --- > > > > yes.c | 17 ++++++++++++----- > > 1 file changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/yes.c b/yes.c > > index dd97ea6..49f5ed6 100644 > > --- a/yes.c > > +++ b/yes.c > > @@ -6,13 +6,20 @@ > > > > int > > main(int argc, char *argv[]) > > { > > > > - char **p; > > + if (argc > 1) { > > + char **p; > > > > - argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0; > > + argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0; > > > > - for (p = argv; ; p = (*p && *(p + 1)) ? p + 1 : argv) { > > - fputs(*p ? *p : "y", stdout); > > - putchar((!*p || !*(p + 1)) ? '\n' : ' '); > > + for (p = argv; ; p = (*p && *(p + 1)) ? p + 1 : argv) { > > + fputs(*p, stdout); > > + putchar((!*p || !*(p + 1)) ? '\n' : ' '); > > + } > > In this case, since we know there is at least one argument, *p is > always true, and I think some simplifications fall out of that. As you said before, the code is hard to follow, so I didn't mess with that part that much. However, in retrospect, the code can be simplfied to this: for (p = argv; ; p = *(p + 1) ? p + 1 : argv) { fputs(*p, stdout); putchar(!*(p + 1) ? '\n' : ' '); } > > + } else { > > + while (1) { > > + fputc('y', stdout); > > + fputc('\n', stdout); > > + } > > I think I'd prefer > > for (;;) > puts("y"); > > here. Originally, I had a puts(3) call there, but this decreased the performance from the unpatched sbase, so I changed it to some fputc calls. Calling write(2) didn't help either. I researched why and it's due to some buffering, but adding buffering would overcomplicate the code. > > } > > > > return 1; /* not reached */ > > > > -- > > 2.21.0 I applied some of your suggestions. The updated diff is below, and some of the code looks much less obfuscated now, as an added benefit. --- yes.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/yes.c b/yes.c index dd97ea6..e1fb4bd 100644 --- a/yes.c +++ b/yes.c @@ -6,13 +6,20 @@ int main(int argc, char *argv[]) { - char **p; + if (argc > 1) { + char **p; - argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0; + argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0; - for (p = argv; ; p = (*p && *(p + 1)) ? p + 1 : argv) { - fputs(*p ? *p : "y", stdout); - putchar((!*p || !*(p + 1)) ? '\n' : ' '); + for (p = argv; ; p = *(p + 1) ? p + 1 : argv) { + fputs(*p, stdout); + putchar(!*(p + 1) ? '\n' : ' '); + } + } else { + for (;;) { + fputc('y', stdout); + fputc('\n', stdout); + } } return 1; /* not reached */ -- 2.21.0 ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Wednesday, June 26, 2019 4:04 AM, Michael Forney <[email protected]> wrote: > What's the motivation for this? Are you running into performance > issues with yes(1), or is this just for fun to increase the throughput > for some benchmark? > > I'd be in favor of changing yes if it increases readability (while > terse, the existing logic is a bit hard to follow), but I don't think > this patch does that. > > On 2019-06-25, AGitBoy [email protected] wrote: > > > yes.c | 17 ++++++++++++----- > > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/yes.c b/yes.c > > index dd97ea6..49f5ed6 100644 > > --- a/yes.c > > +++ b/yes.c > > @@ -6,13 +6,20 @@ > > int > > main(int argc, char *argv[]) > > { > > > > - char **p; > > > > - if (argc > 1) { > > - char **p; > > > > > > > > - argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0; > > > > - argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0; > > > > > > > > - for (p = argv; ; p = (*p && *(p + 1)) ? p + 1 : argv) { > > - fputs(*p ? *p : "y", stdout); > > > > > > - putchar((!*p || !*(p + 1)) ? '\\n' : ' '); > > > > > > > > - for (p = argv; ; p = (*p && *(p + 1)) ? p + 1 : argv) { > > > > > > - fputs(*p, stdout); > > > > > > - putchar((!*p || !*(p + 1)) ? '\\n' : ' '); > > > > > > - } > > > > > > In this case, since we know there is at least one argument, *p is > always true, and I think some simplifications fall out of that. > > > - } else { > > - while (1) { > > > > > > - fputc('y', stdout); > > > > > > - fputc('\\n', stdout); > > > > > > - } > > > > > > I think I'd prefer > > for (;;) > puts("y"); > > here. > > > } > > > > return 1; /* not reached */ > > > > ---------------------------- > > > > 2.21.0
