[Enhancement](memory) Add ConcurrentLong2ObjectHashMap and ConcurrentLong2LongHashMap#61332
Merged
dataroaring merged 7 commits intomasterfrom Mar 31, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds two new segmented-lock concurrent hash maps for FE that use fastutil primitive-key/value maps to reduce memory overhead compared to ConcurrentHashMap<Long, ...> while preserving familiar APIs and providing snapshot-based iteration.
Changes:
- Introduce
ConcurrentLong2ObjectHashMap<V>: concurrent long→object map with per-segment RW locks and atomic compute/merge-style operations. - Introduce
ConcurrentLong2LongHashMap: concurrent long→long map with per-segment RW locks plus an atomicaddTocounter helper. - Add comprehensive JUnit tests for correctness, concurrency behavior, iteration snapshots, and Gson round-trip/format compatibility.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| fe/fe-core/src/main/java/org/apache/doris/common/ConcurrentLong2ObjectHashMap.java | New segmented concurrent long→object map implementation. |
| fe/fe-core/src/main/java/org/apache/doris/common/ConcurrentLong2LongHashMap.java | New segmented concurrent long→long map implementation with addTo. |
| fe/fe-core/src/test/java/org/apache/doris/common/ConcurrentLong2ObjectHashMapTest.java | New unit tests for object map behavior, concurrency, and Gson. |
| fe/fe-core/src/test/java/org/apache/doris/common/ConcurrentLong2LongHashMapTest.java | New unit tests for long map behavior, concurrency, and Gson. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+263
to
+269
| if (seg.map.containsKey(key)) { | ||
| return seg.map.get(key); | ||
| } | ||
| long newValue = mappingFunction.applyAsLong(key); | ||
| seg.map.put(key, newValue); | ||
| return newValue; | ||
| } finally { |
Comment on lines
+451
to
+453
| // Boxed get via Map<Long,Long> interface returns null for missing keys | ||
| Long boxedResult = map.getOrDefault(999L, map.defaultReturnValue()); | ||
| Assertions.assertEquals(0L, boxedResult); |
Comment on lines
+50
to
+51
| * <p><b>Important:</b> All compound operations from both {@link Long2LongMap} and {@link Map} | ||
| * interfaces are overridden to ensure atomicity within a segment's write lock. |
Comment on lines
+26
to
+29
| import it.unimi.dsi.fastutil.longs.LongBinaryOperator; | ||
| import it.unimi.dsi.fastutil.longs.LongOpenHashSet; | ||
| import it.unimi.dsi.fastutil.longs.LongSet; | ||
| import it.unimi.dsi.fastutil.objects.ObjectArrayList; |
...-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2ObjectHashMap.java
Show resolved
Hide resolved
...-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2ObjectHashMap.java
Show resolved
Hide resolved
Comment on lines
+50
to
+51
| * <p><b>Important:</b> All compound operations (computeIfAbsent, computeIfPresent, compute, merge) | ||
| * from both {@link Long2ObjectMap} and {@link Map} interfaces are overridden to ensure atomicity |
Comment on lines
+223
to
+227
| void testNullValues() { | ||
| ConcurrentLong2ObjectHashMap<String> map = new ConcurrentLong2ObjectHashMap<>(); | ||
| map.put(1L, null); | ||
| Assertions.assertTrue(map.containsKey(1L)); | ||
| Assertions.assertNull(map.get(1L)); |
gavinchou
previously approved these changes
Mar 14, 2026
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
dataroaring
commented
Mar 16, 2026
Contributor
Author
dataroaring
left a comment
dataroaring
left a comment
There was a problem hiding this comment.
Thanks for the review! Addressed each comment below:
1. defaultReturnValue divergence (ConcurrentLong2LongHashMap)
Already documented in the class Javadoc (lines 48-50): "The defaultReturnValue is fixed at 0. Calling defaultReturnValue(long) on this wrapper will NOT propagate to the underlying segment maps, and reads/removes will still return 0 for missing keys."
2. Test comment mismatch (ConcurrentLong2LongHashMapTest:453)
The comment on line 454 ("Boxed get via Map<Long,Long> interface returns null for missing keys") applies to line 455 (assertNull(((Map<Long, Long>) map).get(999L))), not to the getOrDefault call above it. The comment is accurate.
3 & 7. Javadoc over-promises atomicity (both classes)
All compound operations from both Long2ObjectMap/Long2LongMap and Map interfaces are overridden:
computeIfAbsent(3 overloads: primitive, Long2*Function, boxed Function)computeIfPresentcomputemergemergeLong(Long2Long only)putIfAbsentreplace(both overloads)remove(Object, Object)
The boxed Map-level variants like putIfAbsent(Long, V) delegate through fastutil's AbstractLong2ObjectMap bridge to our primitive overrides, so they are also atomic. The Javadoc is accurate.
4. Unused imports (LongBinaryOperator, ObjectArrayList)
These imports do not exist in the file. ConcurrentLong2LongHashMap uses java.util.function.LongBinaryOperator fully qualified at line 400. ObjectArrayList is imported and used in ConcurrentLong2ObjectHashMap (line 429). All imports in both files are used.
5 & 6. computeIfAbsent null handling (ConcurrentLong2ObjectHashMap)
Already handled correctly. Both overloads check if (newValue != null) before calling put (lines 282-285 and 301-304), matching the Map.computeIfAbsent contract.
8. Null value semantics
ConcurrentLong2ObjectHashMap already rejects null values — put, putIfAbsent, replace all call Objects.requireNonNull(value) (lines 173, 196, 208, 220). Unit test testNullValuesRejected() (line 224-230) verifies this behavior. This matches ConcurrentHashMap semantics.
github-actions
bot
removed
the
approved
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
2 similar comments
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 26773 ms |
TPC-DS: Total hot run time: 168447 ms |
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 26683 ms |
TPC-DS: Total hot run time: 168565 ms |
morrySnow
reviewed
Mar 23, 2026
Contributor
morrySnow
left a comment
morrySnow
left a comment
There was a problem hiding this comment.
move to fe-foundation module
dataroaring
force-pushed
the
feature/concurrent-fastutil-maps
branch
from
b78116f to
5470db4
Compare
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
…Long2LongHashMap Add thread-safe primitive-key concurrent hash maps built on fastutil, designed to replace ConcurrentHashMap<Long, V> and ConcurrentHashMap<Long, Long> in memory-sensitive FE paths. These maps eliminate Long autoboxing overhead and reduce per-entry memory from ~64 bytes (ConcurrentHashMap) to ~16 bytes, a 4x improvement. Key design: - Segment-based locking (default 16 segments) for concurrent throughput - Full Map interface compatibility for drop-in replacement - Atomic putIfAbsent, computeIfAbsent, replace, remove operations - Comprehensive unit tests covering CRUD, concurrency, iteration, edge cases, and default value semantics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Override defaultReturnValue(long) in ConcurrentLong2LongHashMap to throw UnsupportedOperationException, preventing silent divergence between wrapper and segment maps - Import LongBinaryOperator instead of using fully-qualified name - Tighten Javadoc in both classes to precisely describe atomicity scope - Add test for defaultReturnValue rejection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace HashCommon.mix(long) with inline Murmur3 64-bit finalizer to avoid dependency on specific fastutil version's API - Remove @OverRide from methods that are default on the interface but not present in AbstractLong2LongMap/AbstractLong2ObjectMap, which varies across fastutil versions - Self-implement putIfAbsent/replace/getOrDefault instead of delegating to underlying map methods that may not exist in older versions - Remove explicit fastutil-core dependency since fastutil 8.5.12 is already available transitively via trino-main Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Disambiguate forEach lambda with explicit functional interface cast - Use Long.valueOf() to avoid ambiguous put(Long,V) vs put(long,V) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…module Address reviewer feedback (morrySnow): move ConcurrentLong2ObjectHashMap and ConcurrentLong2LongHashMap from fe-core/common to fe-foundation/util. Add fastutil-core dependency to fe-foundation pom.xml. Tests remain in fe-core as they depend on GsonUtils for serialization tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dataroaring
force-pushed
the
feature/concurrent-fastutil-maps
branch
from
5470db4 to
3fae82b
Compare
Contributor
Author
|
run buildall |
hive-catalog-shade bundles an ancient (~7.x) fastutil without package relocation. When fastutil-core was only a transitive dependency (via fe-foundation), the classloader could pick up the old Long2ObjectOpenHashMap from hive-catalog-shade first, which lacks computeIfAbsent(), causing NoSuchMethodError at runtime. Keeping fastutil-core as a direct dependency ensures it appears before hive-catalog-shade on the classpath. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 26309 ms |
TPC-DS: Total hot run time: 168823 ms |
morrySnow
reviewed
Mar 30, 2026
Contributor
There was a problem hiding this comment.
should move to fe-foundation too
Tests for ConcurrentLong2ObjectHashMap and ConcurrentLong2LongHashMap were in fe-core but the source classes live in fe-foundation. Move them to the correct module and replace GsonUtils.GSON with new Gson() to remove the cross-module dependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 26400 ms |
TPC-DS: Total hot run time: 168750 ms |
morrySnow
approved these changes
Mar 30, 2026
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
mymeiyi
approved these changes
Mar 31, 2026
CalvinKirs
approved these changes
Mar 31, 2026
morningman
pushed a commit
that referenced
this pull request
Apr 2, 2026
…Long2LongHashMap (#61332) ## Summary Add two thread-safe primitive-key concurrent hash maps built on fastutil, designed as drop-in replacements for `ConcurrentHashMap<Long, V>` and `ConcurrentHashMap<Long, Long>` in memory-sensitive FE paths. - **`ConcurrentLong2ObjectHashMap<V>`** — replaces `ConcurrentHashMap<Long, V>` - **`ConcurrentLong2LongHashMap`** — replaces `ConcurrentHashMap<Long, Long>` ### Why `ConcurrentHashMap<Long, V>` costs ~64 bytes per entry due to Long boxing, Node wrapper, and segment overhead. These fastutil-based maps reduce that to ~16 bytes per entry — a **4x memory reduction**. In Doris FE, several critical data structures use `ConcurrentHashMap<Long, V>` at tablet/partition scale (millions of entries), making this a significant memory optimization opportunity. ### Design - **Segment-based locking** (default 16 segments) for concurrent throughput, similar to Java 7's ConcurrentHashMap design - Full `Map` interface compatibility for drop-in replacement - Atomic operations: `putIfAbsent`, `computeIfAbsent`, `replace`, `remove(key, value)` - Thread-safe iteration via snapshot-based `entrySet()`/`keySet()`/`values()` ### Memory comparison | Collection | Per-entry overhead | 1M entries | |------------|-------------------|------------| | `ConcurrentHashMap<Long, V>` | ~64 bytes | ~61 MB | | `ConcurrentLong2ObjectHashMap<V>` | ~16 bytes | ~15 MB | | `ConcurrentHashMap<Long, Long>` | ~80 bytes | ~76 MB | | `ConcurrentLong2LongHashMap` | ~16 bytes | ~15 MB | ## Test plan - [x] `ConcurrentLong2ObjectHashMapTest` — 432 lines covering put/get/remove, putIfAbsent, computeIfAbsent, replace, concurrent writes from multiple threads, iteration consistency, empty map edge cases - [x] `ConcurrentLong2LongHashMapTest` — 455 lines covering CRUD, default value semantics, concurrent operations, atomic operations, iteration, edge cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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
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.
Summary
Add two thread-safe primitive-key concurrent hash maps built on fastutil, designed as drop-in replacements for
ConcurrentHashMap<Long, V>andConcurrentHashMap<Long, Long>in memory-sensitive FE paths.ConcurrentLong2ObjectHashMap<V>— replacesConcurrentHashMap<Long, V>ConcurrentLong2LongHashMap— replacesConcurrentHashMap<Long, Long>Why
ConcurrentHashMap<Long, V>costs ~64 bytes per entry due to Long boxing, Node wrapper, and segment overhead. These fastutil-based maps reduce that to ~16 bytes per entry — a 4x memory reduction.In Doris FE, several critical data structures use
ConcurrentHashMap<Long, V>at tablet/partition scale (millions of entries), making this a significant memory optimization opportunity.Design
Mapinterface compatibility for drop-in replacementputIfAbsent,computeIfAbsent,replace,remove(key, value)entrySet()/keySet()/values()Memory comparison
ConcurrentHashMap<Long, V>ConcurrentLong2ObjectHashMap<V>ConcurrentHashMap<Long, Long>ConcurrentLong2LongHashMapTest plan
ConcurrentLong2ObjectHashMapTest— 432 lines covering put/get/remove, putIfAbsent, computeIfAbsent, replace, concurrent writes from multiple threads, iteration consistency, empty map edge casesConcurrentLong2LongHashMapTest— 455 lines covering CRUD, default value semantics, concurrent operations, atomic operations, iteration, edge cases🤖 Generated with Claude Code