[improve][broker] Optimize SegmentedLongArray with heap-backed long[][]#26183
Conversation
|
This looks like a capacity growth contract issue.
segmentIndex = offset >>> SEGMENT_SHIFT;
indexInSegment = offset & SEGMENT_MASK;With this addressing scheme, every segment before the last one must be fully allocated. Otherwise a logical offset can map into a partial middle segment. For example, with an initial capacity of After Offsets in SegmentedLongArray a = new SegmentedLongArray(3 * 1024 * 1024);
a.increaseCapacity();
a.writeLong(3 * 1024 * 1024, 1L); // within capacity, but failsThere is also a related caller-side issue: It would be safer for the growth API to express the required target capacity, for example: array.ensureCapacity(arrayIdx + ITEMS_COUNT);and have Alternatively, the caller would need to keep growing in a loop until the pending write is guaranteed to fit. |
|
This is a good CPU-side improvement. One concern I have is the GC impact from moving the backing storage from Netty direct memory to Java heap. The logical storage size is roughly the same, but large queues will now use heap It would be good to have an estimate of the GC impact in realistic broker-side scenarios. It may also be worth evaluating whether the existing segment size is still appropriate when the backing storage is on-heap. |
|
Thanks for your GC concern. I analyzed production metric CSV files under realistic broker load (50k msg/s, 30M ledger entries), with full charts attached:
Two seal events captured during a 16-minute window:
|
|
Thanks for addressing the capacity growth issue. The latest version full-allocates every segment in segments[i] = new long[SEGMENT_SIZE];This avoids partial middle segments, but it changes the default memory behavior significantly. A default but the underlying This is expensive for the default path. In bucket delayed delivery there can be multiple There is also a memory accounting issue. while physically allocating a 16 MiB A safer approach would be to keep the initial allocation proportional to the logical initial capacity, and fix the growth issue with an explicit array.ensureCapacity(arrayIdx + ITEMS_COUNT);Then For example: This fixes the segment mapping issue without forcing small/default queues to allocate a 16 MiB heap array upfront, and keeps memory accounting aligned with the actual backing allocation. It may also be worth re-evaluating whether Separately, delayed index memory appears to be exposed mainly as a metric today, rather than being used as a hard limit or backpressure signal. This does not need to be solved in this PR, but if this storage moves on-heap, it may be worth discussing as a follow-up whether delayed tracker memory should have an explicit limit or backpressure policy. |
700cc98 to
a74e6ed
Compare
|
@void-ptr974 Could you have a chance to review this PR? |
|
Thanks for the updates. The current One remaining question is whether Now each full segment is a Also, some unit tests allocate many real segments, e.g. |
|
@void-ptr974 Glad the ensureCapacity(required) approach addresses the default-footprint concern. Regarding SEGMENT_SIZE, I kept the current 2M-long segment size to stay consistent with the previous implementation. The original value was inherited during the migration from ByteBuf to heap arrays, and there is no fundamental correctness requirement that the segment size must remain 2M longs. I agree this is a tunable parameter. Smaller segments can improve allocation granularity, while larger segments reduce the number of segments and resizing operations. The current segment size has been evaluated with our existing workloads and the GC behavior is acceptable. If real-world workloads show allocation pressure or segment management becoming a bottleneck, we can revisit this constant in a separate change with dedicated benchmarks. For the test memory footprint, I added a package-private @VisibleForTesting constructor that allows tests to use a custom segment size while keeping the production default unchanged at 2M longs. Multi-segment grow/shrink tests now use 1024-long segments. The existing seg * 30 coverage is preserved, including the shrink-trim path, while reducing allocation from ~480 MiB to ~240 KiB. |
|
Thanks for sharing the GC data. The 50k msg/s / 30M ledger entries result is useful. I would be interested in the number of consumer groups and the peak delayed backlog per group/broker for that run. Could you also add a few heavier cases — lots of consumer groups with a large backlog, consumer catch-up, and broker recovery? That would make it easier to judge the impact at the upper end. |
@void-ptr974 Regarding the With this change, for the 30M delayed entries case, the full process of sorting the entries and writing them back to BookKeeper can complete within around 30 seconds. The previous implementation had higher overhead due to the buffer-based storage approach. That said, the data structure optimization only addresses the local processing cost. There are still larger architectural optimization opportunities. For example, when multiple consumer groups share the same subscription state, snapshot operations can introduce lock contention. A snapshot may need to wait for another thread holding the lock to finish before proceeding. This is an area we should optimize in the long term by reducing blocking during snapshot and improving concurrency between different operations. So the current change improves the sorting/storage efficiency significantly, while snapshot concurrency and large-scale consumer group scenarios are follow-up areas that need further design and optimization. |
|
Additionally, I think this optimization direction is consistent with the existing approach in the broker codebase. We already use Replacing the buffer-based structure with a primitive-based |
|
Thanks for the clarification. I wasn't questioning the optimization direction. For a pure delayed-delivery topic, every message is tracked independently by each subscription. With the 50k msg/s workload and the default 10-minute minimum ledger rollover interval, one subscription can accumulate roughly 30M delayed entries before the bucket is sealed, which is about 688MiB of queue backing with this implementation. For a topic with 20 independent subscriptions, that would be roughly 13.4GiB of queue backing if they reach the same peak. This is why I was concerned about the per-subscription memory cost at larger scale. |


Motivation
SegmentedLongArraypreviously used NettyByteBufas its backing store. EveryreadLong/writeLongoperation went throughByteBuf.getLong()/setLong(), adding extra indirection on the hot path ofTripleLongPriorityQueueheap operations(siftUp/siftDown).PR #26010 identified
SegmentedLongArrayas the remaining bottleneck after optimizing the heap algorithms, and suggested replacing the backing storage with primitive arrays.This PR replaces the
ByteBufbacking store with segmented heap-backedlong[][]arrays while preserving the segmented architecture required to support capacities beyondInteger.MAX_VALUEelements (PR #16490).Modifications
SegmentedLongArrayByteBufbacking store with segmented heap-backedlong[][]arrays.Benchmark Results
JMH benchmarks from
TripleLongPriorityQueueBenchmark(2 forks, 3 warmup iterations, 5 measurement iterations, JDK 21.0.5).Before (ByteBuf)
After (heap-backed
long[][])Comparison (Size = 2,000,000)