From: Jeff King <peff@peff.net>
To: "René Scharfe" <l.s.r@web.de>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Christian Couder" <christian.couder@gmail.com>,
git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Jonathan Tan" <jonathantanmy@google.com>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [PATCH 15/17] khash: rename oid helper functions
Date: Sun, 23 Jun 2019 18:46:39 -0400 [thread overview]
Message-ID: <20190623224639.GC1100@sigill.intra.peff.net> (raw)
In-Reply-To: <0e4ca2e6-5c75-20c3-def5-b2d3e31f9f08@web.de>
On Sun, Jun 23, 2019 at 06:00:50PM +0200, René Scharfe wrote:
> > I do think if it took the more usual pass-by-const-pointer we'd probably
> > still end up with the exact same code from an optimizing compiler, since
> > all of the references and dereferences would cancel out once it was
> > inlined.
>
> I suspect it depends on the compiler and the exact details. Here's a
> simple experiment: https://godbolt.org/z/kuv4NE. It has a comparison
> function for call by value and one for call by reference as well as a
> wrapper for each with the opposite interface.
>
> An enlightened compiler would emit the same code for functions with the
> same interface, but none of the current ones do in all cases. Clang
> and MSVC do emit the same code for the two call by value functions, so
> there's hope. And moving to call by reference might make matters worse.
> GCC adds some 128-bit moves to both wrappers for some reason.
Hmm. I'm unsure whether your simplified setup is really showing
something interesting or whether we'd need to have true "static inline"
functions that are actually _used_ in a hash table to see if there are
any significant differences.
But...
> Perhaps it doesn't matter much anyway e.g. due to caching, especially
> for the branch-free variants. A definite answer would require
> measurements. Your cleanup would make the necessary surgery on khash.h
> a bit easier by reducing the number of functions and definitions.
Yeah, my gut feeling is it probably doesn't matter much to the overall
operation even if the inner code is slightly different (though I'm happy
to be overridden by real data). And it's definitely something we can
punt on for later (or never if nobody feels like dealing with it).
-Peff
next prev parent reply other threads:[~2019-06-24 2:46 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-15 10:06 [PATCH v4 0/4] Test oidmap Christian Couder
2019-06-15 10:06 ` [PATCH v4 1/4] t/helper: add test-oidmap.c Christian Couder
2019-06-15 10:07 ` [PATCH v4 2/4] t: add t0016-oidmap.sh Christian Couder
2019-06-15 10:07 ` [PATCH v4 3/4] oidmap: use sha1hash() instead of static hash() function Christian Couder
2019-06-15 10:07 ` [PATCH v4 4/4] test-hashmap: remove 'hash' command Christian Couder
2019-06-19 21:42 ` [PATCH v4 0/4] Test oidmap Jeff King
2019-06-19 22:09 ` Jeff King
2019-06-19 22:25 ` Christian Couder
2019-06-20 7:39 ` [PATCH 0/17] drop non-object_id hashing Jeff King
2019-06-20 7:40 ` [PATCH 01/17] describe: fix accidental oid/hash type-punning Jeff King
2019-06-20 16:32 ` Junio C Hamano
2019-06-20 18:25 ` Jeff King
2019-06-20 7:40 ` [PATCH 02/17] upload-pack: rename a "sha1" variable to "oid" Jeff King
2019-06-20 7:40 ` [PATCH 03/17] pack-bitmap-write: convert some helpers to use object_id Jeff King
2019-06-20 7:41 ` [PATCH 04/17] pack-objects: convert packlist_find() " Jeff King
2019-06-20 7:41 ` [PATCH 05/17] pack-objects: convert locate_object_entry_hash() to object_id Jeff King
2019-06-20 7:41 ` [PATCH 06/17] object: convert lookup_unknown_object() to use object_id Jeff King
2019-06-20 7:41 ` [PATCH 07/17] object: convert lookup_object() " Jeff King
2019-06-20 7:41 ` [PATCH 08/17] object: convert internal hash_obj() to object_id Jeff King
2019-06-20 7:41 ` [PATCH 09/17] object: convert create_object() to use object_id Jeff King
2019-06-20 14:21 ` Ramsay Jones
2019-06-20 18:23 ` Jeff King
2019-06-20 7:41 ` [PATCH 10/17] khash: drop broken oid_map typedef Jeff King
2019-06-20 7:41 ` [PATCH 11/17] khash: rename kh_oid_t to kh_oid_set Jeff King
2019-06-20 7:41 ` [PATCH 12/17] delta-islands: convert island_marks khash to use oids Jeff King
2019-06-20 17:38 ` Jonathan Tan
2019-06-20 18:29 ` Jeff King
2019-06-20 7:41 ` [PATCH 13/17] pack-bitmap: convert khash_sha1 maps into kh_oid_map Jeff King
2019-06-20 7:41 ` [PATCH 14/17] khash: drop sha1-specific map types Jeff King
2019-06-20 7:41 ` [PATCH 15/17] khash: rename oid helper functions Jeff King
2019-06-20 17:44 ` Junio C Hamano
2019-06-20 18:27 ` Jeff King
2019-06-23 16:00 ` René Scharfe
2019-06-23 22:46 ` Jeff King [this message]
2019-06-20 7:41 ` [PATCH 16/17] hash.h: move object_id definition from cache.h Jeff King
2019-06-20 17:41 ` Junio C Hamano
2019-06-20 7:41 ` [PATCH 17/17] hashmap: convert sha1hash() to oidhash() Jeff King
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=20190623224639.GC1100@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=avarab@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jonathantanmy@google.com \
--cc=l.s.r@web.de \
--cc=szeder.dev@gmail.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.