Re: [PATCH] Replace hashing algorithm with murmurhash

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 25, 2012 at 10:16:05AM -0700, Eric Blake wrote:
> On 01/25/2012 09:55 AM, Eric Blake wrote:
> >>>> +
> >>>> +    return le32toh(r);
> >>>
> >>> <endian.h>, and thus le32toh(), is not yet standardized (although POSIX
> >>> will be adding it in the future), nor is it currently provided by
> >>> gnulib.  We'd have to get that fixed first.
> >>
> >> The le32toh call was only here because the code I copied wanted to be
> >> endian neutral. I don't think libvirt really cares if its hash codes
> >> are endian neutral, so I trivially just removed the le32toh call and
> >> avoid the problem of endian.h
> > 
> > Agreed - we aren't sharing hash values over the wire, so all hash values
> > within a particular libvirtd process will be the same endianness,
> > without having to waste time on swapping bytes around.
> 
> Actually, the more I think about this, the more I have to wonder:  Does
> the incoming alignment affect the output hash?  That is, if I do
> 
> int i;
> char array[12];
> for (i = 0; i < 4; i++) {
>     strcpy(array + i, "12345678");
>     printf("%x\n", (int) virHashStrCode(array + i, 0));
> }
> 
> do I get the same values for all four iterations, on both little- and
> big-endian architectures?  If not, then the byte-rearranging really is
> important to the algorithm (that is, the algorithm is operating on
> 4-byte quantities, but must build up those quantities from 1-byte
> quantities regardless of starting alignment, so endianness looks like it
> plays a role in doing the conversion correctly).

Consulting the original CPP code, I believe we're fine without it

  https://code.google.com/p/smhasher/source/browse/trunk/MurmurHash3.cpp


// Block read - if your platform needs to do endian-swapping or can only
// handle aligned reads, do the conversion here

FORCE_INLINE uint32_t getblock ( const uint32_t * p, int i )
{
  return p[i];
}

FORCE_INLINE uint64_t getblock ( const uint64_t * p, int i )
{
  return p[i];
}


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]