| Message ID | 555D6537.1090600@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 21, 2015 at 12:55:19AM -0400, Carlos O'Donell wrote: > Oh! You're saying that the performance domain of positional versus > non-positional is different and should not be lumped together, as > Florian did not lump them together in his analysis. > > v2 > - Split out positional vs non-positional. > > Test run: > > "sprintf": { > "positional": { > "duration": 3.49965e+10, > "iterations": 1.7381e+07, > "max": 2733.1, > "min": 2007.32, > "mean": 2013.49 > }, > "non-positional": { > "duration": 3.49927e+10, > "iterations": 2.8668e+07, > "max": 1379.96, > "min": 1214.8, > "mean": 1220.62 > } > } > > 2015-05-21 Carlos O'Donell <carlos@redhat.com> > > * benchtests/Makefile (stdio-common-bench): Define. > (benchset): Add stdio-common-bench. > * sprintf-inputs: New file. > * sprintf-source.c: New file. Looks good to me. Thanks, Siddhesh
On 05/21/2015 06:55 AM, Carlos O'Donell wrote: > Oh! You're saying that the performance domain of positional versus > non-positional is different and should not be lumped together, as > Florian did not lump them together in his analysis. Here's the run with both vfprintf-patched (build) and unpatched (build-master) sources: ==> build/benchtests/bench-sprintf.out <== "sprintf": { "positional": { "duration": 2.4935e+10, "iterations": 1.6674e+07, "max": 1724.24, "min": 1486.34, "mean": 1495.44 }, "non-positional": { "duration": 2.49332e+10, "iterations": 2.5191e+07, "max": 1204.71, "min": 984.675, "mean": 989.767 } } ==> build-master/benchtests/bench-sprintf.out <== "sprintf": { "positional": { "duration": 2.49366e+10, "iterations": 1.4544e+07, "max": 1948.33, "min": 1709.47, "mean": 1714.57 }, "non-positional": { "duration": 2.49334e+10, "iterations": 2.3758e+07, "max": 1277.08, "min": 1042.82, "mean": 1049.47 } } Again, the numbers are suspiciously good. But at least they do not show a performance regression. I'll commit the remaining vfprintf patches then.
On 05/21/2015 10:02 AM, Florian Weimer wrote: > Again, the numbers are suspiciously good. But at least they do not show > a performance regression. Agreed. At least the benchmarks allow interested parties to test roughly the same test on their hardware and complain if it made it slower for them. The fact that sprintf got faster is awesome, and isn't entirely far fetched given the cleanup. > I'll commit the remaining vfprintf patches then. Please do. Thanks. Cheers, Carlos.
On 05/21/2015 12:55 AM, Carlos O'Donell wrote: > On 05/21/2015 12:16 AM, Siddhesh Poyarekar wrote: >> On Wed, May 20, 2015 at 11:40:17PM -0400, Carlos O'Donell wrote: >>> 2015-05-20 Carlos O'Donell <carlos@redhat.com> >>> >>> * benchtests/Makefile (stdio-common-bench): Define. >>> (benchset): Add stdio-common-bench. >>> * sprintf-inputs: New file. >>> * sprintf-source.c: New file. >>> >>> diff --git a/benchtests/Makefile b/benchtests/Makefile >>> index cb7a97e..8e615e5 100644 >>> --- a/benchtests/Makefile >>> +++ b/benchtests/Makefile >>> @@ -48,7 +48,9 @@ include ../gen-locales.mk >>> >>> stdlib-bench := strtod >>> >>> -benchset := $(string-bench-all) $(stdlib-bench) >>> +stdio-common-bench := sprintf >>> + >>> +benchset := $(string-bench-all) $(stdlib-bench) $(stdio-common-bench) >>> >>> CFLAGS-bench-ffs.c += -fno-builtin >>> CFLAGS-bench-ffsll.c += -fno-builtin >>> diff --git a/benchtests/sprintf-inputs b/benchtests/sprintf-inputs >>> new file mode 100644 >>> index 0000000..0e034b5 >>> --- /dev/null >>> +++ b/benchtests/sprintf-inputs >>> @@ -0,0 +1,8 @@ >>> +## args: char *:const char *:int:char:char:char:char:char:const char *:float:unsigned int >>> +## ret: int >>> +## includes: stdio.h >>> +## include-sources: sprintf-source.c >>> +# Test positional arguments: >>> +buf, FORMAT1, 1001, '1', '2', '3', '4', '5', "string", 1.5, 0x1234 >>> +# Test non-positional arguments: >>> +buf, FORMAT2, 1001, '1', '2', '3', '4', '5', "string", 1.5, 0x1234 >> >> You would want two different outputs for positional and non-positional >> respectively to match Florian's output. You can achieve that with the >> ##name directive by putting the line: > > Oh! You're saying that the performance domain of positional versus > non-positional is different and should not be lumped together, as > Florian did not lump them together in his analysis. > > v2 > - Split out positional vs non-positional. > > Test run: > > "sprintf": { > "positional": { > "duration": 3.49965e+10, > "iterations": 1.7381e+07, > "max": 2733.1, > "min": 2007.32, > "mean": 2013.49 > }, > "non-positional": { > "duration": 3.49927e+10, > "iterations": 2.8668e+07, > "max": 1379.96, > "min": 1214.8, > "mean": 1220.62 > } > } > > 2015-05-21 Carlos O'Donell <carlos@redhat.com> > > * benchtests/Makefile (stdio-common-bench): Define. > (benchset): Add stdio-common-bench. > * sprintf-inputs: New file. > * sprintf-source.c: New file. Checked in. c.
diff --git a/benchtests/Makefile b/benchtests/Makefile index cb7a97e..8e615e5 100644 --- a/benchtests/Makefile +++ b/benchtests/Makefile @@ -48,7 +48,9 @@ include ../gen-locales.mk stdlib-bench := strtod -benchset := $(string-bench-all) $(stdlib-bench) +stdio-common-bench := sprintf + +benchset := $(string-bench-all) $(stdlib-bench) $(stdio-common-bench) CFLAGS-bench-ffs.c += -fno-builtin CFLAGS-bench-ffsll.c += -fno-builtin diff --git a/benchtests/sprintf-inputs b/benchtests/sprintf-inputs new file mode 100644 index 0000000..9a7710d --- /dev/null +++ b/benchtests/sprintf-inputs @@ -0,0 +1,10 @@ +## args: char *:const char *:int:char:char:char:char:char:const char *:float:unsigned int +## ret: int +## includes: stdio.h +## include-sources: sprintf-source.c +## name: positional +# Test positional arguments: +buf, FORMAT1, 1001, '1', '2', '3', '4', '5', "string", 1.5, 0x1234 +## name: non-positional +# Test non-positional arguments: +buf, FORMAT2, 1001, '1', '2', '3', '4', '5', "string", 1.5, 0x1234 diff --git a/benchtests/sprintf-source.c b/benchtests/sprintf-source.c new file mode 100644 index 0000000..fc125a5 --- /dev/null +++ b/benchtests/sprintf-source.c @@ -0,0 +1,6 @@ +/* A set of arbitrarily selected positional format specifiers. */ +#define FORMAT1 " %1$d: %2$c%3$c%4$c%5$c%6$c %7$20s %8$f (%9$02x)\n" +/* A matching, but arbitrarily selected, set of non-positional format specifiers. */ +#define FORMAT2 " %d: %c%c%c%c%c %20s %f (%02x)\n" +/* Sufficiently large buffer. */ +char buf[256];