feat(WIP): add IntMapByDynamicHash #2294
Merged
Conversation
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
Codecov Report
@@ Coverage Diff @@ ## master #2294 +/- ## ============================================ - Coverage 65.07% 64.00% -1.08% - Complexity 979 987 +8 ============================================ Files 498 502 +4 Lines 41240 42304 +1064 Branches 5738 5982 +244 ============================================ + Hits 26837 27075 +238 - Misses 11743 12502 +759 - Partials 2660 2727 +67
... and 18 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
javeme reviewed Aug 30, 2023
| } | ||
| | ||
| @SuppressWarnings("UnusedDeclaration") | ||
| private volatile int size; // updated via atomic field updater |
| /* | ||
| we want 7 extra slots and 64 bytes for each | ||
| slot. int is 4 bytes, so 64 bytes is 16 ints. | ||
| */ |
| } | ||
| } | ||
| | ||
| private static class Entry { |
Contributor
There was a problem hiding this comment.
we expect a primitive type instead of a class, it's too many objects because each Entry will be generated an object
| AtomicReferenceFieldUpdater.newUpdater(IntMapByDynamicHash.class, Entry[].class, | ||
| "table"); | ||
| | ||
| private volatile Entry[] table; |
Contributor
There was a problem hiding this comment.
expect a primitive type array here
Contributor
| Is it still in progress? |
conghuhu changed the title 
Contributor Author
Yes, I'm still working on it |
javeme reviewed Sep 14, 2023
| } | ||
| | ||
| @SuppressWarnings("UnusedDeclaration") | ||
| private volatile int size; // updated via atomic field updater |
Contributor
There was a problem hiding this comment.
move to the front of the class
| } | ||
| | ||
| private static int tableSizeFor(int c) { | ||
| int n = c - 1; |
Contributor
There was a problem hiding this comment.
rename c/n to readable words?
| | ||
| @Override | ||
| public void clear() { | ||
| long[] currentArray = this.table; |
| while (true) { | ||
| int length = currentArray.length; | ||
| int index = this.hash(extractKey(toCopyEntry), length); | ||
| long o = IntMapByDynamicHash2.tableAt(currentArray, index); |
| } | ||
| | ||
| private void reverseTransfer(long[] src, ResizeContainer resizeContainer) { | ||
| long[] dest = resizeContainer.nextArray; |
| private void resize(long[] oldTable, int newSize) { | ||
| int oldCapacity = oldTable.length; | ||
| int end = oldCapacity - 1; | ||
| long last = IntMapByDynamicHash2.tableAt(oldTable, end); |
| * Enable the current thread to participate in the expansion | ||
| */ | ||
| private long[] helpWithResize(long[] currentArray) { | ||
| if (resizeContainer == null) { |
| } | ||
| } | ||
| | ||
| private long[] helpWithResizeWhileCurrentIndex(long[] currentArray, int index) { |
| return this.slowRemove(key, currentTable) != NULL_VALUE; | ||
| } | ||
| | ||
| long e = o; |
Contributor
There was a problem hiding this comment.
also add a fastRemove method?
| if (o == RESIZED || o == RESIZING) { | ||
| return this.slowGet(key, currentTable); | ||
| } | ||
| long e = o; |
Contributor
| seems run rocksdb raft ci failed with timehout 6h, same with: https://github.com/apache/incubator-hugegraph/actions/runs/6470766980/usage?pr=2278 |
imbajin approved these changes Oct 26, 2023
Member
imbajin left a comment
imbajin left a comment There was a problem hiding this comment.
Merge it now & address the comment & problems later
conghuhu added a commit to conghuhu/incubator-hugegraph that referenced this pull request Dec 5, 2023
simon824 pushed a commit that referenced this pull request Dec 12, 2023
* feat(WIP): add IntMapByDynamicHash (#2294) * feat: add values & keys in IntMapByDynamicHash * add some basic comment & fix some style * feat: fix pr review * fix: fix some review --------- Co-authored-by: imbajin <jin@apache.org>
VGalaxies pushed a commit to VGalaxies/incubator-hugegraph that referenced this pull request Jan 12, 2024
* feat(WIP): add IntMapByDynamicHash (apache#2294) * feat: add values & keys in IntMapByDynamicHash * add some basic comment & fix some style * feat: fix pr review * fix: fix some review --------- Co-authored-by: imbajin <jin@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 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.
Purpose of the PR
Compared to jdk's concurrent HashMap, the effect has been improved:
Main Changes
New dynamic hash implementation for intMap, ensuring concurrency security.
Verifying these changes
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need