github-actions[bot] commented on code in PR #68564:
URL: https://github.com/apache/doris/pull/68564#discussion_r4120483275
##########
be/src/exec/pipeline/dependency.h:
##########
@@ -461,6 +461,12 @@ struct BucketedAggSharedState : public BasicSharedState {
/// Element i is true when instance i's data for this bucket has been
merged.
/// Sized to num_sink_instances in init_instances().
std::vector<bool> merged_instances;
+ /// Arena for memory allocated by aggregate function merges into this
bucket.
+ /// Accessed only under merge_in_progress CAS lock. Sink instance
arenas cannot be
+ /// used here because different buckets are merged concurrently by
different
+ /// source instances and Arena is not thread-safe. Merged states may
point into
+ /// this arena, so it must live as long as the shared state.
+ Arena arena;
Review Comment:
[P2] Avoid a 4 KiB chunk for every populated merge bucket. `Arena` lazily
allocates its default 4 KiB chunk on the first insertion, and
`collect_set(string)` inserts during each merge. If a small merged value
reaches each of the 256 buckets, these arenas retain at least 1 MiB per shared
state even when the copied strings occupy only a few kilobytes; the old path
reused sink-instance arenas. This multiplies under query concurrency. Please
use a merge allocation layout with a smaller per-bucket floor or fewer
independently owned arenas while preserving source-task exclusivity and
shared-state lifetime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
