Optimize DataSetWithoutTimeGenerator with primitive long heap/set - #906
Conversation
Replace PriorityQueue<Long>/HashSet<Long> with fastutil-derived LongHeapPriorityQueue/LongOpenHashSet to avoid boxing on the multi-way merge hot path. Update NOTICE and add regression tests.
There was a problem hiding this comment.
Pull request overview
This PR optimizes the multi-way merge hot path in DataSetWithoutTimeGenerator by replacing boxed timestamp containers (PriorityQueue<Long> / HashSet<Long>) with fastutil-derived primitive long heap/set implementations to reduce autoboxing overhead and GC pressure in query workloads.
Changes:
- Introduces fastutil 8.5.8–derived primitive containers:
LongHeapPriorityQueueandLongOpenHashSet. - Switches
DataSetWithoutTimeGeneratorto use the primitive heap/set, pre-sized by series count. - Adds NOTICE attribution and regression tests covering merge correctness, data-structure correctness, and a (currently unguarded) micro-benchmark.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| NOTICE | Adds required attribution for fastutil-derived code. |
| java/tsfile/src/main/java/org/apache/tsfile/utils/LongOpenHashSet.java | New primitive open-addressing long hash set (trimmed from fastutil). |
| java/tsfile/src/main/java/org/apache/tsfile/utils/LongHeapPriorityQueue.java | New primitive long min-heap priority queue (trimmed from fastutil). |
| java/tsfile/src/main/java/org/apache/tsfile/read/query/dataset/DataSetWithoutTimeGenerator.java | Replaces boxed heap/set with primitive implementations and pre-sizes them. |
| java/tsfile/src/test/java/org/apache/tsfile/read/query/dataset/DataSetWithoutTimeGeneratorTest.java | Adds regression tests for sparse multi-way merge and cross-batch continuation. |
| java/tsfile/src/test/java/org/apache/tsfile/utils/LongOpenHashSetAndHeapPriorityQueueTest.java | Adds correctness tests plus a micro-benchmark/perf smoke test for the new containers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| private void timeHeapPut(long time) { | ||
| if (!timeSet.contains(time)) { | ||
| timeSet.add(time); | ||
| timeHeap.add(time); | ||
| timeHeap.enqueue(time); | ||
| } |
There was a problem hiding this comment.
Agreed, switched to if (timeSet.add(time)) to avoid the double probe.
| public void testPrimitiveStructuresPerformance() { | ||
| final int series = 50; | ||
| final int rows = 100_000; | ||
| final long baseTime = 1_700_000_000_000L; | ||
|
|
||
| runBoxed(series, 10_000, baseTime); | ||
| runPrimitive(series, 10_000, baseTime); | ||
|
|
There was a problem hiding this comment.
Agreed, gated with Assume.assumeTrue behind -Dtsfile.runPerformanceTests=true, same as other TsFile perf tests.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #906 +/- ##
===========================================
+ Coverage 60.60% 60.71% +0.11%
===========================================
Files 745 747 +2
Lines 49508 49678 +170
Branches 7945 7979 +34
===========================================
+ Hits 30002 30163 +161
- Misses 18046 18055 +9
Partials 1460 1460 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| ============================================================================ | ||
|
|
||
| This product includes code derived from fastutil (http://fastutil.di.unimi.it/): | ||
| Copyright (C) 2002-2022 Sebastiano Vigna | ||
| Copyright (C) 2003-2022 Paolo Boldi and Sebastiano Vigna | ||
| Licensed under the Apache License, Version 2.0 |
Summary
Optimize the multi-way merge hot path in
DataSetWithoutTimeGeneratorby replacingPriorityQueue<Long>/HashSet<Long>with trimmed, fastutil 8.5.8–derived primitive containers, avoiding autoboxing of timestamps and the related CPU/GC cost.CPU profiles showed the hotspot on
HashSet.contains→HashMap.getNodeviatimeHeapPut, which is hit frequently in workloads such as Pipe extraction.Changes
LongHeapPriorityQueueandLongOpenHashSet(forked from fastutil, APIs trimmed to what TsFile needs; key methods annotated with upstream origins)DataSetWithoutTimeGeneratorto these containers, with capacity pre-sized by series countNOTICEfor the fastutil-derived code and copyrightsDataSetWithoutTimeGeneratorTest: sparse multi-way merge and cross-batch continuationLongOpenHashSetAndHeapPriorityQueueTest: heap/set correctness (including edge cases), parity vs boxed JDK collections, combined dedup-merge pattern, and a micro-benchmarkPerformance results
Micro-benchmark of the same heap+set dedup-merge pattern used by
DataSetWithoutTimeGenerator(50 series × 100,000 rows), comparing JDKPriorityQueue<Long>+HashSet<Long>with the primitive implementation:PriorityQueue+HashSet)LongHeapPriorityQueue+LongOpenHashSet)Notes: micro-benchmarks vary with machine load; local runs typically land around ~2x–6x. Absolute gain depends on series count and total points. Correctness tests vs boxed behavior all passed.
Test plan
mvn -pl tsfile test -Dtest=LongOpenHashSetAndHeapPriorityQueueTest,DataSetWithoutTimeGeneratorTestLICENSEcomplianceHashSet.containshotspot shrinks