All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH 2/5] hashmap: allow memihash computation to be continued
Date: Mon, 20 Feb 2017 12:27:27 -0800	[thread overview]
Message-ID: <xmqqwpckip5c.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1702201342020.3496@virtualbox> (Johannes Schindelin's message of "Mon, 20 Feb 2017 13:43:38 +0100 (CET)")

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

>> 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.


  reply	other threads:[~2017-02-20 20:27 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-14 11:31 [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area Johannes Schindelin
2017-02-14 11:31 ` [PATCH 1/5] name-hash: eliminate duplicate memihash call Johannes Schindelin
2017-02-14 11:32 ` [PATCH 2/5] hashmap: allow memihash computation to be continued Johannes Schindelin
2017-02-18  5:35   ` Junio C Hamano
2017-02-20 12:43     ` Johannes Schindelin
2017-02-20 20:27       ` Junio C Hamano [this message]
2017-02-14 11:32 ` [PATCH 3/5] name-hash: precompute hash values during preload-index Johannes Schindelin
2017-02-18  5:47   ` Junio C Hamano
2017-02-19  0:19     ` Jeff Hostetler
2017-02-19 21:45       ` Junio C Hamano
2017-02-14 11:32 ` [PATCH 4/5] name-hash: specify initial size for istate.dir_hash table Johannes Schindelin
2017-02-14 11:32 ` [PATCH 5/5] name-hash: remember previous dir_entry during lazy_init_name_hash Johannes Schindelin
2017-02-14 22:03 ` [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area Jeff King
2017-02-15 14:27   ` Jeff Hostetler
2017-02-15 16:44     ` Jeff King
2017-02-18  5:56       ` Junio C Hamano
2017-02-19  0:02         ` Jeff Hostetler
2017-02-18  5:58     ` Junio C Hamano
2017-02-18  6:29       ` Jeff King
2017-02-18 20:48         ` Junio C Hamano
2017-02-18 23:52           ` Jeff Hostetler
2017-02-19 21:50             ` Junio C Hamano
2017-03-02 21:11     ` Junio C Hamano
2017-03-02 21:18       ` Jeff Hostetler
2017-03-02 21:40         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwpckip5c.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=jeffhost@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.