Re: [PATCH 2/5] hashmap: allow memihash computation to be continued

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2017, 20:27 UTC
Message-ID: <xmqqwpckip5c.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1702201342020.3496@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 7 quoted lines

>> If an extra call level really matters, its "inline" equivalent in
>> the header would probably be good.
>
> Well, the hashing is supposed to be as fast as possible, so I would like
> to avoid that extra call level. However, the end result is not so pretty
> because FNV32_BASE needs to be made public (OTOH it removes more lines
> than it adds):

I think our usual answer is "can we measure the difference to demonstrate that the overhead for an extra call matter?"

As two functions sit next to each other in a single file, the code duplication does not bother me _that_ much. A single liner

    /* keep implementations of these two in sync */

in front of these two functions would not hurt, but whoever attempts to come up with a better hash needs to stare at this file carefully anyway, so lack of such carefulness probably wouldn't be too big an issue, either.

But the above 8 lines are something we need to worry about after we definitely know that we MUST have two independent functions that are supposed to be kept in sync; a patch that makes us worry them before we know is a premature optimization, and that bothers me even more than the actual code duplication that can drift apart.