Re: [PATCH 1/1] compat: modernize and simplify byte swapping functions
- From
- Rostislav Krasny <rostiprodev@gmail.com>
- Date
- Jan 2, 2026, 17:37 UTC
- Message-ID
- <CAKU3Xk5=dmdQhTgHB8WrPbbOOo3cyJtCgFgo7juW06F9YaceRQ@mail.gmail.com>
- In-Reply-To
- <20260102061626.GA2581074@coredump.intra.peff.net>
On Fri, 2 Jan 2026 at 08:16, Jeff King <peff@peff.net> wrote:
Show 9 quoted lines
> > On Fri, Jan 02, 2026 at 02:27:35AM +0200, Rostislav Krasny wrote: > > > Replace manual bit operations with memcpy + network functions for better > > maintainability. Add missing 16-bit network byte order conversion. > > This is burying the lede a bit, as they say. I don't know that the > maintainability is much changed, especially as these are not functions > that anybody looks at or touches very often.
I just meant that the code has become simpler and more understandable. English is not my native language, but if you would like me to rephrase the commit message, I'll be happy to do so. Just suggest a better wording and I will send v2.
Show 15 quoted lines
> But this part might be compelling: > > > - Performance improvements (GCC 15.2.1, Clang 21.1.7): > > * on x86-64 with -O0 4.2x faster (GCC), 3.7x faster (Clang) > > * on x86-64 with -O1 4x faster (GCC), identical (Clang) > > * on x86-64 with -O2 identical (GCC), 1.8x faster (Clang) > > The -O0 numbers are IMHO not very interesting (and are entirely > expected; you are comparing optimized library memcpy versus unoptimized > assignments). But clang making -O2 faster is quite interesting. > > If we are going to do this, I think it would be for the improved > performance. And it would be nice for the commit message to go into > details about what was measured and how. I'll respond elsewhere in the > thread with some more thoughts.
The main motivation of this pull request is improved simplicity, readability and consistency of the new code. It looked strange to me that for converting a value the optimized ntoh* and hton* macros from the same bswap.h could be used, while for pointers a more complicated code in the get_be* and put_be* functions is used. This PR also brings a few more small improvements like fix of typos in names and additional functions for API completeness.
My performance tests (that you discuss in your second email) show that at least there is no regression in performance. They also show that with n < 2 in -On it is much easier for the compiler to optimize the new code. I can agree that -O0 and -O1 are less relevant in release builds of Git, but the fact that the new code makes the compiler's work easier with any -On only strengthens the assertion that the new code has no performance loss.
And yes, the code that I used to measure the performance of get_be64() is probably not ideal but at least it proves that there is no regression in performance.
> -Peff