From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37406) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anwQ9-0004rb-Vv for qemu-devel@nongnu.org; Wed, 06 Apr 2016 18:59:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anwQ6-0003RC-KH for qemu-devel@nongnu.org; Wed, 06 Apr 2016 18:59:33 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:54828) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anwQ6-0003R6-E4 for qemu-devel@nongnu.org; Wed, 06 Apr 2016 18:59:30 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id B531C21260 for ; Wed, 6 Apr 2016 18:59:29 -0400 (EDT) Date: Wed, 6 Apr 2016 18:59:29 -0400 From: "Emilio G. Cota" Message-ID: <20160406225929.GA15573@flamenco> References: <1459834253-8291-1-git-send-email-cota@braap.org> <1459834253-8291-7-git-send-email-cota@braap.org> <87lh4rarup.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87lh4rarup.fsf@linaro.org> Subject: Re: [Qemu-devel] [PATCH 06/10] include: add xxhash.h List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex =?iso-8859-1?Q?Benn=E9e?= Cc: MTTCG Devel , Peter Maydell , Peter Crosthwaite , QEMU Developers , Sergey Fedorov , Paolo Bonzini , Richard Henderson On Wed, Apr 06, 2016 at 12:39:42 +0100, Alex Bennée wrote: > > Emilio G. Cota writes: > > > xxhash is a fast, high-quality hashing function. The appended > > brings in the 32-bit version of it, with the small modification that > > it assumes the data to be hashed is made of 32-bit chunks; this increases > > speed slightly for the use-case we care about, i.e. tb-hash. > > > > The original algorithm, as well as a 64-bit implementation, can be found at: > > https://github.com/Cyan4973/xxHash > > > > Signed-off-by: Emilio G. Cota > > --- > > include/qemu/xxhash.h | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 106 insertions(+) > > create mode 100644 include/qemu/xxhash.h > > > > diff --git a/include/qemu/xxhash.h b/include/qemu/xxhash.h > > new file mode 100644 > > index 0000000..a13a665 > > --- /dev/null > > +++ b/include/qemu/xxhash.h > > @@ -0,0 +1,106 @@ > > > + > > +/* u32 hash of @n contiguous chunks of u32's */ > > +static inline uint32_t qemu_xxh32(const uint32_t *p, size_t n, uint32_t seed) > > +{ > > What is the point of seed here? I looked on the original site to see if > there was any guidance on tuning seed but couldn't find anything. I > appreciate the compiler can inline the constant away but perhaps we > should #define it and drop the parameter if we are not intending to > modify it? The seed value would only matter if we needed consistent hashes (we don't). Any seed value should give similar performance, given xxhash's quality. I'll set a define for this seed if it is used in more than one place. > Also it might be helpful to wrap the call to avoid getting the > boilerplate sizing wrong: > > #define qemu_xxh32(s) qemu_xxh32_impl((const uint32_t *)s, sizeof(*s)/sizeof(uint32_t), 1) > > Then calls become a little simpler for the user: > > return qemu_xxh32(&k); > > Do we need to include a compile time check for structures that don't > neatly divide into uint32_t chunks? I'll give the original byte-sized function a shot and come back to this if necessary. Thanks, E.