feat(WIP): add IntMapByDynamicHash#2294
Merged
imbajin merged 3 commits intoapache:tmp-mapfrom Oct 26, 2023
Merged
Conversation
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: |
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 <[email protected]>
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 <[email protected]>
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
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