Manually unbox closure in Hashtbl.mem#11500
Merged
nojb merged 3 commits intoocaml:trunkfrom Aug 22, 2022
Merged
Conversation
This avoids closure allocation and GC call in mem. Numerous other Hashtbl functions are already like this, particularly find from d48c6cf.
nojb
approved these changes
Aug 22, 2022
Contributor
nojb
left a comment
There was a problem hiding this comment.
LGTM
Maybe a Changes entry would be in order: "Make Hashtbl.mem non-allocating".
Contributor
Author
Added. |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This avoids closure allocation and the associated GC call in
mem. Numerous otherHashtblfunctions (remove,find,find_opt,replace) are already like this. In particular, this was done tofindfor non-allocation reasons way back in d48c6cf.I don't have any data for performance change one way or another. A while back while
perf-ing someHashtblheavy code, I noticed GC calls inmem, which was unexpected as this lookup shouldn't need any allocation, and after some digging realized it's themem_in_bucketclosure causing that.