On Mon, 2012-01-09 at 13:52 -0500, Simo Sorce wrote:
On Mon, 2012-01-09 at 13:37 -0500, Stephen Gallagher wrote:
> On Mon, 2012-01-09 at 13:36 -0500, Simo Sorce wrote:
> > On Mon, 2012-01-09 at 13:27 -0500, Simo Sorce wrote:
> > > On Mon, 2012-01-09 at 12:58 -0500, Stephen Gallagher wrote:
> > > > On Tue, 2012-01-03 at 18:00 -0500, Simo Sorce wrote:
> > > > > >From [PATCH 0/0] A shared memory cache to perform better:
> > > > >
> > > > > 0/3: New utility functions/headers
> > > > > These are also quite straightforward but they are useless
without the
> > > > > later patches
> > > >
> > > > Nack.
> > > >
> > > > Your implementation doesn't match the original:
> > > >
> > > >
> > > > From MurmurHash3_x86_32():
> > > > h1 = h1*5+0xe6546b64;
> > > >
> > > > From this patch:
> > > > h1 = h1 + 5 + 0xe6546b64;
> > > >
> > > > I can only assume that s/*/+ is not going to yield proper results :)
> > >
> > > OUPS, well it will give *different* results, but consistent.
> > > Anyway I'll fix it and send back a fixed patch.
> > >
> > > >
> > > > I fail to understand why you use (nblocks * 4) instead of
'len', given
> > > > that nblocks was defined as len/4
> > >
> > > Because (nblocks * 4) != len and it is 3 times out 4 < len :-)
> > > Remember nblocks is an integer, so any fractional part is lost, which is
> > > what we want here, we want to process len - (nblocks * 4) remaining
> > > bytes.
> > >
> > > > While I know that it's a fixed value, I'd rather see
sizeof(uint32_t)
> > > > instead of '4' in getblock() (for readability and clarity)
> > >
> > > Ok.
> >
> > Attached a patch with the requested code changes addressed.
>
>
> I think you resent the old patch
I think you are right.
Let's try again.
Ok new revision.
First patch just fixes one building warning in the testes.
Second patch adds a new tests for checking that the hash returns the
same value (deterministic) and it does it by hashing each time a random
value so that we always test the function with a different value and
have greater coverage (may catch if specific values cause the hash
function to fail).
--
Simo Sorce * Red Hat, Inc * New York