Re: [PATCH 15/17] khash: rename oid helper functions

From: Jeff King <peff@peff.net>
Date: Jun 23, 2019, 22:46 UTC
Message-ID: <20190623224639.GC1100@sigill.intra.peff.net>
In-Reply-To: <0e4ca2e6-5c75-20c3-def5-b2d3e31f9f08@web.de>

On Sun, Jun 23, 2019 at 06:00:50PM +0200, René Scharfe wrote:

Show 15 quoted lines

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