Re: [PATCH] diff: use strncmp to check for "diff." prefix

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Dec 19 2022, Rose via GitGitGadget wrote:

> From: Seija Kijin <doremylover123@gmail.com>
>
> This would be a lot more efficient than
> checking for the full string, as
> so many of the accepted values
> begin with "diff."
>
> This allows the computer to skip most checks
> if var did not start with diff at all.
>
> It would also let us add more "diff." strings
> in the future without having to duplicate
> these 5 character so many times.

This is missing a "why", is this one-off really needed for optimization
purposes? If not it doesn't seem worth the churn.

That we duplicate the "diff." part of "diff.renames" (as opposed to
"renames" is also a feature in my opinion, it makes things easier to
grep for.

It would really just be preferrable to have the compiler do this work
for us. A while ago I experimented with that, i.e. if/else if/else
chains of strcmp(): https://godbolt.org/z/rrnYWW7nj

As that example shows at least GCC doesn't do anything interesting with
it as optimizations are concerned, i.e. you'd really want something like
this instead (but for the compiler to do it for you):
https://godbolt.org/z/7hsPo59TM

But, as you can be seen at https://godbolt.org/z/zWK46rGK5 it *will* do
that if we just use memcmp().

So, doing that really seems preferrable, perhaps with some macro that
would provide the "n" to memcmp, so we wouldn't need to hardcode the
length.

But all of that would be predicated on whether the optimization is
actually worth it.

If we are micro-optimizing config reading we should be moving the
relevant code to the configset API, where we have the parsed keys as a
hash, rather than trying to micro-optimize the callback API, where we'll
call the callback N times, just to brute-force ask it if it's interested
in some small subset of the config space.