-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](fe) modify TabletInvertedIndex to reduce memory #60715
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
base: master
Are you sure you want to change the base?
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
2a3448e to
72ff315
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the TabletInvertedIndex classes to use fastutil's Long2ObjectOpenHashMap instead of Guava's HashMap and HashBasedTable to reduce memory consumption. The fastutil library provides memory-efficient primitive collections that avoid boxing overhead when using primitive types as keys.
Changes:
- Replaced
HashMap<Long, V>withLong2ObjectOpenHashMap<V>in TabletInvertedIndex and CloudTabletInvertedIndex - Replaced
HashBasedTable(a nested map structure) with nestedLong2ObjectOpenHashMapin LocalTabletInvertedIndex - Added fastutil-core 8.5.18 dependency to the project
- Updated LICENSE file to reflect the new fastutil version
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| fe/fe-core/pom.xml | Adds fastutil-core 8.5.18 dependency for memory-efficient primitive collections |
| dist/LICENSE-dist.txt | Updates fastutil version reference from 6.5.6 to 8.5.18 |
| fe/fe-core/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java | Replaces HashMap with Long2ObjectOpenHashMap for tabletMetaMap |
| fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTabletInvertedIndex.java | Replaces HashMap with Long2ObjectOpenHashMap for replicaMetaMap |
| fe/fe-core/src/main/java/org/apache/doris/catalog/LocalTabletInvertedIndex.java | Replaces HashBasedTable with nested Long2ObjectOpenHashMap structures, updates all related methods to handle the new data structure, and adds proper cleanup of empty nested maps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <!-- fastutil for memory-efficient primitive collections --> | ||
| <dependency> | ||
| <groupId>it.unimi.dsi</groupId> | ||
| <artifactId>fastutil-core</artifactId> |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential mismatch between the artifact declared in pom.xml ("fastutil-core") and the LICENSE file ("fastutil"). The standard fastutil artifact is typically "it.unimi.dsi:fastutil", not "fastutil-core". The "fastutil-core" is a modular variant introduced in fastutil 8.x. Please verify that "fastutil-core" is the correct artifact to use, and if so, ensure the LICENSE file reflects the actual artifact name consistently. If using the standard "fastutil" artifact instead, update the pom.xml accordingly.
| <artifactId>fastutil-core</artifactId> | |
| <artifactId>fastutil</artifactId> |
|
run buildall |
TPC-H: Total hot run time: 30442 ms |
|
|
||
| // tablet id -> tablet meta | ||
| protected Map<Long, TabletMeta> tabletMetaMap = Maps.newHashMap(); | ||
| protected Long2ObjectOpenHashMap<TabletMeta> tabletMetaMap = new Long2ObjectOpenHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any quantitive comparison with the previous impl.?
TPC-DS: Total hot run time: 189822 ms |
ClickBench: Total hot run time: 28.47 s |
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)