Skip to content

Comment hash function in pos_initial#11650

Closed
dra27 wants to merge 1 commit intoocaml:trunkfrom
dra27:addrmap-hash
Closed

Comment hash function in pos_initial#11650
dra27 wants to merge 1 commit intoocaml:trunkfrom
dra27:addrmap-hash

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Oct 20, 2022

Addresses a comment from the multicore merge in #10861.

It may be possible/desirable to improve on this in a future release.

Co-authored-by: Stephen Dolan <sdolan@janestreet.com>
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Oct 20, 2022

This PR is the result of a side-channel discussion with @stedolan - it just documents the current state, which should be fine for 5.0. A separate issue can be opened if wanted to track investigating improvements to it.

@xavierleroy
Copy link
Copy Markdown
Contributor

If this hash function was a known design described and evaluated in the literature, a comment pointing to the description and evaluation would be useful. A comment that just says "we picked this function on a whim and have no idea how well it works" is not useful, so I'll close this PR. I make a mental note to replace this code by a well-known hash function soon (but not in 5.0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants