Re: [PATCH v2] x86-64: Optimize bzero

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 23/02/2022 05:12, Noah Goldstein wrote:
> On Tue, Feb 15, 2022 at 7:38 AM Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>>
>> Hi,
>>
>>> Is there any way it can be setup so that one C impl can cover all the
>>> arch that want to just leave `__memsetzero` as an alias to `memset`?
>>> I know they have incompatible interfaces that make it hard but would
>>> a weak static inline in string.h work?
>>
>> No that won't work. A C implementation similar to current string/bzero.c
>> adds unacceptable overhead (since most targets just implement memset and
>> will continue to do so). An inline function in string.h would introduce target
>> hacks in our headers, something we've been working hard to remove over the
>> years.
>>
>> The only reasonable option is a target specific optimization in GCC and LLVM
>> so that memsetzero is only emitted when it is known an optimized GLIBC
>> implementation exists (similar to mempcpy).
>>
>>> It's worth noting that between the two `memset` is the cold function
>>> and `__memsetzero` is the hot one. Based on profiles of GCC11 and
>>> Python3.7.7 setting zero covers 99%+ cases.
>>
>> There is no doubt memset of zero is by far the most common. What is in doubt
>> is whether micro-optimizing is worth it on modern cores. Does Python speed up
>> by a measurable amount if you use memsetzero?
> 
> Ran a few benchmarks for GCC/Python3.7
> 
> There is no measurable benefit using '__memsetzero' in Python3.7
> 
> For GCC there are some cases where there is a consistent speedup
> though it's not universal.
> 
> Times are geomean (N=30) of memsetzero / memset
> (1.0 means no difference, less than 1 means improvement, greater than
> 1 regression).
> 
>  Size, N Funcs,  Type, memsetzero / memset
> small,       1, bench,             0.99986
> small,       1, build,             0.99378
> small,       1,  link,             0.99241
> small,      10, bench,             0.99712
> small,      10, build,             0.99393
> small,      10,  link,             0.99245
> small,     100, bench,             0.99659
> small,     100, build,             0.99271
> small,     100,  link,             0.99227
> small,     250, bench,             1.00195
> small,     250, build,             0.99609
> small,     250,  link,             0.99744
> large,     N/A, bench,             0.99930
> 
> 
> The "small" size basically means the file was filled with essentially empty
> functions i.e
> ```
> int foo(void) { return 0; }
> ```
> 
> N Funcs refers to the number of these functions per file, so small-250 would
> be 250 empty functions per file.
> 
> Bench recompiled the same file 100x times
> Build compiled all the files
> Link linked all the files with a main that emitted 1x call per function
> 
> The "large" size was a realistic file someone might compile (in this case
> a freeze of sqlite3.c).
> 
> The performance improvement for the build/link step for varying amount of
> small functions per file was consistently in the ~.8% range. Not mind blowing
> but I believe its a genuine improvement.
> 
> I don't think this shows expected GCC usage is going to be faster, but
> I do think it shows that the effects of this change could be noticeable in an
> application.
> 

I hardly consider this marginal improvement a good reason to add a libc
symbol, specially because it is unlikely most architecture will ever provide
an optimized version of this (aarch64 maintainer said there are not looking
forward to it) and newer architecture extensions (such as s390 mvcle) or 
compile optimizations (such PGO or LTO) might remove the function call
altogether.

> NB: I'm not exactly certain why 'bench' doesn't follow the same trend
> as build/link.
> The only thing I notice is 'bench' takes longer (implemented in a Makefile
> loop) so possibly to '+ c' term just dampens any performance differences.
> The math for this doesn't work out 100% so there is a bit to still be skeptical
> of.