Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yves/remove stadtx #18569

Merged
merged 2 commits into from Feb 13, 2021
Merged

Yves/remove stadtx #18569

merged 2 commits into from Feb 13, 2021

Conversation

demerphq
Copy link
Collaborator

Nuff said.

git ls-files returns the list of currently staged files. If a patch
is aimed at removing a file, thus deleting it and removing it from
MANIFEST but the dev has not staged the changes we should not say it
fails test.
@demerphq demerphq merged commit 65c0a6d into blead Feb 13, 2021
@Leont
Copy link
Contributor

Leont commented Feb 13, 2021

Nuff said.

For sake of posterity, can you please actually explain here why this is necessary.

@demerphq
Copy link
Collaborator Author

I wrote Stadtx to be fast, but i overlooked some real statistical issues in how it works. For certain keys it will be far less well distributed than preferable. meaning more bucket splits than necessary, more linked list searches than necessary, and less efficient utilization of available memory. The perl hash data structure degrades relatively well under these conditions, but the tiny speedup of the actual string hashing compared to the other costs isn't worth it. Having less collisions per bucket mitigates the speed differences to a certain extent also. Siphash produces well distributed data, hashes reasonably fast (especially in the 1-3 variant) and is one of the few widely used and peer reviewed PRF's. Stadtx is a project i came up with to see if I could create a fast hash function. There isn't even a question in terms of statistical quality comparison. If we want to provide a "fast" hash then someone else can select one from the herd, but Stadtx shouldnt be an option.

@demerphq
Copy link
Collaborator Author

Also I should mention that Stephen Dolan did some quality research into this. I had actually discovered the same issues independently but he did a much more formal investigation. Credit goes to him for instigating this removal.

@jkeenan
Copy link
Contributor

jkeenan commented Feb 13, 2021 via email

@Leont
Copy link
Contributor

Leont commented Feb 13, 2021

Or why it was rushed through so quickly?

Yes, this. Don't get me wrong, I think this removal is the right decision, but you should at least give the rest of us a chance to come to that conclusion before merging it.

@rjbs rjbs deleted the yves/remove_stadtx branch May 8, 2021 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants