search

ninja-build / ninja

a small build system with a focus on speed
https://ninja-build.org/
Apache License 2.0
11.26k stars 1.6k forks source link

Ninja command hashing could use the newer MurmurHash3 function #636

Open rgeary1 opened 11 years ago

rgeary1 commented 11 years ago

Ninja computes the command line hash with MurmurHash2 (from the comments). There is a newer, faster MurmurHash algorithm, MurmurHash3 https://code.google.com/p/smhasher/wiki/MurmurHash3

Based on the quoted performance stats, MurmurHash3_x64_128 should take 66% of the time of MurmurHash2. Also MurmurHash2 had a flaw in the algorithm.

nico commented 11 years ago

Do you have benchmarks showing how much faster it is? I believe it's at least significantly longer, which is a minus.

The flaw was that it's possible to engineer collisions as far as I know, which isn't a problem for ninja's use case.

rgeary1 commented 11 years ago

I don't have any numbers other than the performance quoted in the link (which is actually from an old version of the algorithm). I can implement it, shouldn't be hard.

jonesmz commented 4 years ago

@jhasse Help Wanted?

jhasse commented 4 years ago

I wouldn't be able to review it, sry.

jonesmz commented 4 years ago

I meant, add the label to the issue on github so people understand that the maintainers of ninja won't work on this themselves.

jhasse commented 4 years ago

If the label was "we won't work on this", I would add it. But "help wanted" sounds like "if you write a PR we will be happy to review and merge it" to me. But I wouldn't have the time to review a PR for this, so I don't want to give people wrong impressions.

jonesmz commented 4 years ago

If you're not going to accept a PR for an improvement (which, as you point out, implies that someone needs to review it), then close the damn bug. It's 6 years old. Just having the bug open actively indicates that the project considers it something worthy of action.

jhasse commented 4 years ago

Closing would be the wrong message IMHO. Tags like "low priority" or "backlog" might be fitting.