Improvements to LFHT #26
Open
Conversation
This file contains 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
|
Hi! Sorry last month was really busy. Are you closing this because of inactivity or because those changes are not relevant anymore ? |
added 3 commits
October 22, 2024 16:27
First, a hung resize loop when out of memory. Next, non-atomic use of resize_initiated which resulted in resize stopping completely when a queued work item was run too soon.
Just one pointer to buckets is needed in the mmap backend and often less than 64 pointers are needed in the order backend (unless "infinite").
|
A few initial comments on the PR:
For the mutex leak, please describe which code path appears to leak it. For the variable size cds_lfht, we should consider using a flexible array at the end if the size is now truly variable. |
There was a bug in one of my commits found in pre-prod testing. Before fixing it I closed the PR because I was not sure how many more are there. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The earliest commit adds error checking to the memory allocations done by cds_lfht_new, so that NULL is returned instead of triggering an assertion. For compatibility with existing apps I've sprinkled public functions with assert(ht). As for resizes, they are now interrupted when out of memory. This is not perfect--the table may remain too small for too long--but better than dumping core.
Another commit reduces the size of struct cds_lfht. These things matter to me as my application creates tons of hash tables dynamically.
Two more commits pertain to call_rcu and workqueue. One fixes a mutex leak on FreeBSD (I gather the mutex is not needed there at all). Another one is a minor tweak to set cpu affinity as early as possible, not 255 iterations later.