[auto-perf-opt] Reuse protobuf scratch buffers in KeyValueMerger to cut PK-index compaction CPU#72434
Closed
luohaha wants to merge 1 commit into
Closed
[auto-perf-opt] Reuse protobuf scratch buffers in KeyValueMerger to cut PK-index compaction CPU#72434luohaha wants to merge 1 commit into
luohaha wants to merge 1 commit into
Conversation
… PK-index compaction CPU
Profile of cloud_native_pk_index_compact under realtime-benchmark
999_1gb_1_1tb workload (perf -F 99 -e cpu-clock 60s on the hottest
CN, 6 nodes, 32 cores each, all 6 saturated at 99-100% non-idle CPU)
shows my_free at 38.6% self-time with the dominant call paths inside
KeyValueMerger::merge -> KeyValueMerger::flush and inside
~IndexValuesWithVerPB destruction. Inclusive view:
78.91% LakePersistentIndexParallelCompactTask::run
55.26% KeyValueMerger::merge
26.59% KeyValueMerger::flush
9.63% my_free (dtor of local index_value_pb)
9.62% operator new (RepeatedField growth)
4.92% ~IndexValuesWithVerPB
1.76% sstable::TableBuilder::Add
11.62% my_free (dtor of local index_value_ver)
11.18% MessageLite::ParseFromString (10.72% in operator new)
5.45% ~IndexValuesWithVerPB
Both `merge()` and `flush()` allocate a fresh IndexValuesWithVerPB on
the stack per call and let it destruct at scope exit, freeing all of
the protobuf's internal RepeatedField storage even though the next
call needs an identically-shaped message. With one merger per
compact task and one merge() call per merged key, this is millions
of malloc/free pairs per task — at the per-task exec time of 46s
observed in iteration 001, allocator churn alone accounts for the
majority of compaction CPU.
Fix: hoist the two protobuf messages plus the SerializeAsString
output buffer to KeyValueMerger members, and Clear() / SerializeToString()
into them per call. Clear() preserves RepeatedField capacity, so
subsequent ParseFromString reuses already-grown internal arrays
instead of going back to the allocator. KeyValueMerger is constructed
once per LakePersistentIndexParallelCompactTask and is single-threaded
within the task, so no synchronization is needed.
Expected impact: based on the profile, removing the in-loop allocator
churn should shave 30-50% CPU off cloud_native_pk_index_compact tasks,
reducing per-task exec time and freeing CPU for the publish_version /
async_delta_writer / cloud_native_pk_index_execution pools that share
the same cores. Iteration 002 of auto-perf-opt my-rt-2 will deploy
and measure.
Related: supersedes StarRocks#72428, which capped the compact thread count at
num_cores. Throttling a CPU-bound pool that is doing real work makes
read-amplification worse over time; making each unit of work cheaper
is the correct fix.
Signed-off-by: auto-perf-opt <casey.luo@celerdata.com>
|
auto-perf-opt seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
2 tasks
This was referenced May 2, 2026
Closed
19 tasks
Contributor
Author
|
Superseded by upstream/main #72843 (merged). Closing this branch-4.1 draft. |
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
Cloud-native PK-index compaction (
cloud_native_pk_index_compact) burns ~38% of all BE/CN CPU inmy_free(jemalloc) under sustained upsert workloads. A 60-secondperfcapture on the hottest CN of anauto-perf-opt my-rt-2999_1gb_1_1tb run (6×32-core CNs, all at 99-100% non-idle CPU) traces it back to per-key allocator churn insidestarrocks::lake::KeyValueMerger:Both
merge()andflush()allocate a freshIndexValuesWithVerPBon the stack on every call and let the destructor free all of the protobuf's internalRepeatedFieldstorage at scope exit. The merger is constructed once per compact task and called once per key from the merging iterator — millions of malloc/free pairs per task. At the per-task exec time of 46.6 s observed inauto-perf-opt my-rt-2iteration 001, allocator churn alone accounts for the majority of compaction CPU.Change
Hoist the two
IndexValuesWithVerPBinstances and theSerializeAsString()output buffer ontoKeyValueMergeras members, andClear()/SerializeToString()into them per call. Clear() preservesRepeatedFieldcapacity, so subsequentParseFromStringreuses already-grown internal arrays instead of going back to the allocator. The merger is single-threaded within a task, so no synchronization is needed.Files:
be/src/storage/lake/lake_persistent_index_key_value_merger.{h,cpp}— 2 files, +15 / -4 lines.Expected impact
cloud_native_pk_index_compactper-task exec time.publish_version/async_delta_writer/cloud_native_pk_index_executionfinish faster, cutting theparallel_upsert_wait_uscomponent of slow publishes (2.5-2.9 s of a 3.1 s total in iter-001).Relationship to #72428
This PR supersedes #72428. That PR proposed capping the compact thread count at
num_cores()(i.e. dropping 128/CN → 32/CN), which would have throttled a CPU-bound pool doing real work — slashing PK-index compaction throughput 4×, letting SSTs accumulate, and making upserts/queries worse over time. The correct fix is to make each unit of compaction work cheaper, which is what this PR does. #72428 should be closed once this lands.Test plan
benchmark_luoyixin_1tb_6cn_2disk2.999_1gb_1_1tbtemplate; measure:cloud_native_pk_index_compactper-task exec time (target: < 30 s vs. 46.6 s baseline).queue_countshould NOT grow unbounded vs. baseline (rollback signal).perfprofile; verifymy_freeself-time falls well below 38 %, and~IndexValuesWithVerPBno longer dominates.🤖 Generated with auto-perf-opt via Claude Code (my-rt-2 iter-002)