lib: vsprintf: added a put_dec() test and benchmark tool
in [Kernel]
|
Prev: [PATCH 39/43] memblock: Export MEMBLOCK_ERROR
Next: [PATCH 07/43] memblock: Introduce for_each_memblock() and new accessors
From: Denys Vlasenko on 6 Aug 2010 01:20 On Friday 06 August 2010 00:38, Michal Nazarewicz wrote: > This commit adds a test application for the put_dec() and > family of functions that are used by the previous two commits. > > Signed-off-by: Michal Nazarewicz <mina86(a)mina86.com> > +put-dec-test: put-dec-test.c > + exec $(CC) $(CFLAGS) -o $@ $< (1) Why exec? (2) Add -Wall, you'd be surprised > +static uint64_t rand_64(void) > +{ > + uint64_t v = 0, m = 1; > + for (;;) { > + uint64_t n; > + v = m * rand(); > + n = m + m * RAND_MAX; > + if (n < m) > + break; > + m = n; > + } > + return v; > +} What this function do? Looks cryptic. In my testing, it picks 0 quite often. > +static char buf1[24]; Can you size the array safely, without assuming that long long is no wider than 64 bits? > +#define FUNC(func, outer, inner, correction, format, value) do { \ > + struct timeval start; \ > + unsigned i, o; \ > + for (i = (inner); i--; ) { \ > + typeof(value) v = (value); \ > + ret |= test(#func, func(buf1, v), format, v); \ I'd add memset(buf1, 77, sizeof(buf1)) before test > +int main(int argc, char **argv) { > + unsigned long iterations = 1000, i; > + struct timeval correction; > + int ret = 0; > + > + srand(time(NULL)); > + > + if (argc > 1) > + iterations = atoi(argv[1]); > + > + gettimeofday(&correction, NULL); > + for (i = 25000 * iterations; i; --i) > + rand_64(); > + stop(NULL, &correction, NULL); Why is this "correction" thing needed? I looked at the entire machinery and I see no reason to have it. > + puts(">> Benchmarks:\n\tput_dec_full()"); > + fflush(NULL); > + > + FUNC(orig_put_dec_full, iterations, 100000, NULL, "%05u", i); You have variable named i, you pass it as macro parameter, but macro has local variable named i too. Is it an International Obfuscated C Code Contest entry? > + FUNC(mod1_put_dec_full, iterations, 100000, NULL, "%05u", i); > + FUNC(mod3_put_dec_full, iterations, 100000, NULL, "%05u", i); > + FUNC(mod5_put_dec_full, iterations * 10, 10000, NULL, "%04u", i); > + puts("\tput_dec_trunc()"); > + fflush(NULL); > + > + FUNC(orig_put_dec_trunc, iterations, 100000, NULL, "%u", i); > + FUNC(mod1_put_dec_trunc, iterations, 100000, NULL, "%u", i); > + FUNC(mod3_put_dec_trunc, iterations, 100000, NULL, "%u", i); > + FUNC(mod5_put_dec_trunc, iterations * 10, 10000, NULL, "%u", i); > + FUNC(mod3_put_dec_8bit, iterations * 500, 256, NULL, "%u", i); > + puts("\n\tput_dec()"); > + fflush(NULL); > + > + FUNC(orig_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); "%llu" fmt is for unsigned long long, not uint64_t. > + FUNC(mod1_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); > + FUNC(mod2_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); > + FUNC(mod3_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); > + FUNC(mod4_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); > + FUNC(mod5_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); > + FUNC(mod6_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); > + FUNC(mod7_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); > + FUNC(mod8_put_dec, iterations / 4, 100000, &correction, "%llu", rand_64()); Here a lot of CPU time is taken by rand() calls. Also, you use different values for different functions, which is wrong. -- vda -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Raja R Harinath on 6 Aug 2010 12:00 Hi, Michał Nazarewicz <m.nazarewicz(a)samsung.com> writes: > On Fri, 06 Aug 2010 07:10:06 +0200, Denys Vlasenko <vda.linux(a)googlemail.com> wrote: > >> On Friday 06 August 2010 00:38, Michal Nazarewicz wrote: >>> This commit adds a test application for the put_dec() and >>> family of functions that are used by the previous two commits. >>> >>> Signed-off-by: Michal Nazarewicz <mina86(a)mina86.com> >> >>> +put-dec-test: put-dec-test.c >>> + exec $(CC) $(CFLAGS) -o $@ $< >> >> (1) Why exec? > > Micro Makefile optimisation -- saves us a fork(). I don't think it's worth it. The use of 'exec' will force make to invoke the shell, while without the exec, make has an opportunity to optimize out the shell invocation completely [1]. Forcing the use of a shell to avoid a fork() sounds like a terrible trade-off. - Hari [1] See, for instance, construct_command_argv_internal() in http://cvs.savannah.gnu.org/viewvc/make/job.c?root=make&view=markup -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Denys Vlasenko on 6 Aug 2010 15:30 On Friday 06 August 2010 10:34, MichaÅ Nazarewicz wrote: > On Fri, 06 Aug 2010 07:10:06 +0200, Denys Vlasenko <vda.linux(a)googlemail.com> wrote: > > > On Friday 06 August 2010 00:38, Michal Nazarewicz wrote: > >> This commit adds a test application for the put_dec() and > >> family of functions that are used by the previous two commits. > >> > >> Signed-off-by: Michal Nazarewicz <mina86(a)mina86.com> > > > >> +put-dec-test: put-dec-test.c > >> + exec $(CC) $(CFLAGS) -o $@ $< > > > > (1) Why exec? > > Micro Makefile optimisation -- saves us a fork(). > > I'll try to fix the benchmark over the weekend and will post updated > version. Thanks for the comments. You might find some ideas in the attached file: * removed "correction" code * added verification of correctness for put_dec() * different rand64 (old one was giving same "random" number surprisingly often) * more robust coding in a few places -- vda
From: Michal Nazarewicz on 6 Aug 2010 17:00
Denys Vlasenko <vda.linux(a)googlemail.com> writes: > On Friday 06 August 2010 10:34, MichaÅ Nazarewicz wrote: >> On Fri, 06 Aug 2010 07:10:06 +0200, Denys Vlasenko <vda.linux(a)googlemail..com> wrote: >> >> > On Friday 06 August 2010 00:38, Michal Nazarewicz wrote: >> >> This commit adds a test application for the put_dec() and >> >> family of functions that are used by the previous two commits. >> >> >> >> Signed-off-by: Michal Nazarewicz <mina86(a)mina86.com> >> > >> >> +put-dec-test: put-dec-test.c >> >> + exec $(CC) $(CFLAGS) -o $@ $< >> > >> > (1) Why exec? >> >> Micro Makefile optimisation -- saves us a fork(). >> >> I'll try to fix the benchmark over the weekend and will post updated >> version. Thanks for the comments. > > You might find some ideas in the attached file: > * removed "correction" code > * added verification of correctness for put_dec() > * different rand64 > (old one was giving same "random" number surprisingly often) > * more robust coding in a few places Thanks! I actually changed the benchmark earlier today and redid all benchmarks but I'll sure incorporate you're test code. Also, I've removed rand_64() from my code in favour of reading random data from /dev/urandom. In consequence, all functions ale benchmarked using the same values and it's still random (ie. no the same value all the time). This also made "correction" no longer needed. It's 11pm here, so I'll try to send the new patches tomorrow morning after getting some sleep. Once again, thank you for all the comments and suggestiveness! -- Best regards, _ _ .o. | Liege of Serenly Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michal "mina86" Nazarewicz (o o) ooo +--<mina86-tlen.pl>--<jid:mina86-jabber.org>--ooO--(_)--Ooo-- |