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

perf: Remove unused naturalJoin key states. #5770

Merged
merged 46 commits into from
Jul 21, 2024

Conversation

cpwright
Copy link
Contributor

@cpwright cpwright commented Jul 13, 2024

The generated hash tables for naturalJoin (and other operations) never removed a bucket's hash table entry when it became entry. For cases where a naturalJoin cycles through buckets, the hash table could be filled with empty values consuming memory without reason.

This changes the generated open addressed hash table for both-sides incremental naturalJoin to mark entries deleted with a tombstone when both the left and right side are empty. The occupancy of the hash table for use in load factor computation includes the number of entries in the hash table and tombstones. The rehash operation may copy entries into a new table that is the same size as the original table instead of always doubling in size, allowing tombstone entries to be reaped.

Closes #5769

@cpwright cpwright changed the title Remove unused naturalJoin key states. perf: Remove unused naturalJoin key states. Jul 13, 2024
@rcaudy rcaudy self-requested a review July 16, 2024 14:49
@rcaudy rcaudy added ReleaseNotesNeeded Release notes are needed and removed NoReleaseNotesNeeded No release notes are needed. labels Jul 16, 2024
@rcaudy rcaudy added this to the 0.36.0 milestone Jul 16, 2024
cpwright and others added 4 commits July 20, 2024 06:02
…raljoin/IncrementalNaturalJoinStateManagerTypedBase.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
…raljoin/typed/incopen/gen/IncrementalNaturalJoinHasherChar.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
Copy link
Member

@rcaudy rcaudy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can approve as-is. There are a few open comments, though. Maybe worth addressing the one about the unnecessary alternateLiveEntries?

@cpwright cpwright merged commit ec36842 into deephaven:main Jul 21, 2024
16 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

naturalJoin should be able to reap key states with no responsive left or right rows
2 participants