HappenLee commented on code in PR #68500: URL: https://github.com/apache/doris/pull/68500#discussion_r4374816677
########## be/src/exec/operator/analytic_spill.h: ########## @@ -0,0 +1,129 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include <cstddef> +#include <cstdint> +#include <memory> +#include <vector> + +#include "common/status.h" +#include "core/block/block.h" +#include "core/custom_allocator.h" +#include "exprs/aggregate/aggregate_function.h" + +namespace doris { + +class RuntimeProfile; +class RuntimeState; +class SpillFile; +class SpillFileWriter; + +using SpillFileSPtr = std::shared_ptr<SpillFile>; +using SpillFileWriterSPtr = std::shared_ptr<SpillFileWriter>; + +/// An immutable run of consecutive analytic partitions handed from the sink to the source. Rows +/// keep the input order. partition_ends[p] is the exclusive end row of partition p inside the batch +/// and peer_group_ends are batch-relative. Small partitions are batched together so that the source +/// replays ordinary sized Blocks even when every partition contains only a few rows. +struct AnalyticSpillBatch { + struct RecordMemory { + size_t serialized_bytes = 0; + size_t decoded_bytes = 0; + size_t footer_bytes = 0; + }; + + int64_t rows = 0; + std::vector<Block> blocks; + SpillFileSPtr data_file; + RecordMemory data_record_memory; + + DorisVector<int64_t> peer_group_ends; + SpillFileSPtr peer_group_file; + RecordMemory peer_record_memory; + + std::vector<int64_t> partition_ends; + std::vector<WindowSpillStrategy> strategies; + std::vector<WindowSpillPeerFunction> peer_functions; + // Row p holds the result of partition p for PARTITION_REDUCE functions. + std::vector<ColumnPtr> partition_results; + // Element p holds the parameter of partition p for PARTITION_CARDINALITY functions. + std::vector<std::vector<int64_t>> function_parameters; Review Comment: Could we remove the per-partition NTILE parameter arrays rather than only change their allocator? FE already requires each NTILE bucket argument to be a positive integer literal before translation (`Ntile.checkLegalityAfterRewrite()`), so a given function has the same bucket count in every partition. `_finish_spill_partition()` currently appends that identical value for each partition, and replay reads it through `function_parameters[function_index][_partition_index]`. Please retain one bucket count per NTILE function, carry it once in the batch descriptor, and read it by `function_index` during replay. This would remove the per-partition append and indexing as well as the redundant `_open_partition_parameters` / `_batch_function_parameters` state. Partition boundaries and per-partition reduction results still need their existing storage. The parameter payload would shrink from O(functions * partitions) to O(functions): for 32 NTILE functions and 65,535 partitions, approximately 16 MiB becomes 256 bytes. This also removes the large untracked buffers identified above. Please cover multiple NTILE functions with different bucket counts across several partitions, including in-memory and spilled replay. This suggestion is based on static inspection of af6a8034f5aa; I have not built or run it. -- 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]
