[PATCH 3/3] resolv: Optimize inet_ntop
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Mon Jun 9 20:15:21 GMT 2025
On 06/06/25 13:51, Maciej W. Rozycki wrote:
> On Wed, 4 Jun 2025, Adhemerval Zanella Netto wrote:
>
>>>> + if (word >= 100)
>>>> ...
>>>> + if (word >= 10)
>>>
>>> Since this is for performance, shouldn't the code avoid conditional branches? Something like the following, say:
>>>
>>> static inline char *
>>> put_uint8 (uint8_t word, char *tp)
>>> {
>>> *tp = '0' + word / 100;
>>> tp += 100 <= word;
>>> *tp = '0' + word / 10 % 10;
>>> tp += 10 <= word;
>>> *tp++ = '0' + word % 10;
>>> return tp;
>>> }
>>>
>>
>> I am not sure, with gcc 14.2.1 this version is indeed faster on aarch64 (Neoverse N1),
>> but slower on x86_64 (Zen3):
>
> How many samples did you collect?
For both x86_64 and aarch64 I collected on an idle system that I assured
no extra OS jitter would interfere in the result, and I don't see much
variability over multiple runs (I will need to do some statistics analysis
to give a proper number).
As for other benchmark, it should not matter performance variability due
hardware frequency scaling, and we do have some handling in bench-strings
for this case.
>
> Hmm, I suppose on a modern write-back D$ system consecutive plain writes
> to the same memory location will be coalesced. But do all the systems do
> write allocation nowadays? I think this approach asks for a prefetch hint
> at the start of the function.
>
> I have benchmarked this alternative:
>
> static inline char *
> put_uint8 (uint8_t word, char *tp)
> {
> unsigned int o, t, h;
>
> h = word;
> o = h % 10;
> h /= 10;
> t = h % 10;
> h /= 10;
>
> *tp = '0' + h;
> tp += !!h;
> *tp = '0' + t;
> tp += !!t;
> *tp++ = '0' + o;
>
> return tp;
> }
>
> on my POWER9 system and it produces results similar to Paul's proposal
> (taking single samples only; there's some variation between runs and I
> didn't bother taking the average as that'd require some manual effort):
>
> "inet_ntop_ipv4": {
> "workload-ipv4-random": {
> "duration": 5.36267e+08,
> "iterations": 3.6e+07,
> "reciprocal-throughput": 14.8943,
> "latency": 14.8984,
> "max-throughput": 6.714e+07,
> "min-throughput": 6.71215e+07
> }
> }
>
> (Paul's) vs:
>
> "inet_ntop_ipv4": {
> "workload-ipv4-random": {
> "duration": 5.38168e+08,
> "iterations": 3.6e+07,
> "reciprocal-throughput": 14.9995,
> "latency": 14.8988,
> "max-throughput": 6.66691e+07,
> "min-throughput": 6.71196e+07
> }
> }
>
> (mine). If your benchmarking turns out good with this proposal, then I
> can obtain a representative set of samples for POWER9.
The results of your suggestion is similar to Paul's on the hardware I tested
(x86_64/Zen3, AArch64/Neoverse-N1, powerpc/POWER10):
reciprocal-throughput patch suggestion improvement
x86_64/zen3 26.1951 37.4577 -43.00%
x86_64/Westmere [1] 94.3904 89.9536 4.70%
i686/zen3 46.5747 57.4518 -23.35%
aarch64 19.8964 16.8005 15.56%
power10 9.43355 7.11195 24.61%
So it really seems an issue with the generated code and Zen3. I also checked
with a lower gcc version than I used (13 vs 14), but the x86_64 result are
the same. Nor '-march=znver3 -mtune=znver3' made any difference also.
I tried to check if there is any perf counter to help understand this issue,
but it seems that Core::X86::Pmc::Core::Store_Globally_Visible_Cancels_2
is only available to newer Zen core (to check if this is relate to write-back
consecutive plain writes hazards).
[1] cfarm187 from gcc compile farm.
>
> I can't benchmark Aarch64 or x86-64 easily, but I note that my proposal
> produces fewer multiplications for both plus lets x86-64 take advantage of
> the carry flag via SBB instead of using a discrete conditional-set+add
> sequence, so chances are it'll perform better. It produces more compact
> code too, and I think it might be more readable for some though YMMV.
>
> Though the dependency on the quality of the optimiser seems very fragile
> here, e.g. if the locals are changed to a signed data type, then POWER9
> produces an extra instruction that causes performance to drop by ~2.5%,
> but x86-64 is able to convert another conditional-set+add to SBB, which
> for a change likely causes a performance gain.
I am even more inclined to use with my current version below mainly because
it shows less surprising results on different chips. And we already getting
a 10x improvement over current implementation, so we can fine-tune this even
more once we figure out why AMD chips are behaving like this.
static inline char *
put_uint8 (uint8_t word, char *tp)
{
int s = 1;
if (word >= 10)
{
if (word >= 100)
{
tp[2] = '0' + word % 10;
word /= 10;
s += 1;
}
tp[1] = '0' + word % 10;
word /= 10;
s += 1;
}
*tp = '0' + word;
return tp + s;
}
>
> Also I think you need to fold the update quoted below into 1/3, so as to
> let the newly-added group of benchmarks be run on its own, or one gets an
> error otherwise:
>
> $ make BENCHSET=bench-resolv bench
> The following values in BENCHSET are invalid: bench-resolv
> The valid ones are: bench-math bench-pthread bench-string calloc-simple calloc-tcache calloc-thread hash-benchset malloc-simple malloc-tcache malloc-thread math-benchset stdio-benchset stdio-common-benchset stdlib-benchset string-benchset wcsmbs-benchset
> Makefile:485: *** Invalid BENCHSET value. Stop.
Ack, I will fix it.
>
> Maciej
>
> ---
> benchtests/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> glibc-bench-resolv.diff
> Index: glibc/benchtests/Makefile
> ===================================================================
> --- glibc.orig/benchtests/Makefile
> +++ glibc/benchtests/Makefile
> @@ -462,6 +462,7 @@ ifneq ($(strip ${BENCHSET}),)
> VALIDBENCHSETNAMES := \
> bench-math \
> bench-pthread \
> + bench-resolv \
> bench-string \
> calloc-simple \
> calloc-tcache \
>
More information about the Libc-alpha
mailing list