Re: [PATCH 5/6] vfprintf: Introduce printf_positional function

["Carlos O'Donell" <carlos at redhat dot com>]

On 03/06/2015 07:57 AM, Florian Weimer wrote:
> On 03/01/2015 10:16 PM, Florian Weimer wrote:
>> This splits a considerable chunk of code from the main vfprintf
>> function.  This will make it easier to remove the use of extend_alloca
>> from the positional argument handling code.
> 
> Inspection of the generated assembly on x86_64 shows that splitting the
> two functions helps GCC 4.9 with register allocation; there are fewer
> spills.
> 
> I used the following totally made-up benchmark to see if there is a
> performance regression.

I converted your made-up benchmark into a benchtest test.

Care to test this with your patches and see if it shows a difference?

I'd say if it shows no difference, then checkin your changes, and I'll
checkin this benchmark to prevent regressions in sprintf, despite it
being trivial.

Example results before were (i5-4690K):
  "sprintf": {
   "": {
    "duration": 3.49974e+10,
    "iterations": 2.162e+07,
    "max": 2077.29,
    "min": 1214.74,
    "mean": 1618.75
   }
  }


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
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, non-positional format specifiers.  */
+#define FORMAT2 "   %d: %c%c%c%c%c %20s %f (%02x)\n"
+/* Sufficiently large buffer.  */
+char buf[256];
---

Cheers,
Carlos.