gianm commented on code in PR #19353: URL: https://github.com/apache/druid/pull/19353#discussion_r3264292861
########## processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/HeapVectorGrouper.java: ########## @@ -0,0 +1,218 @@ +/* + * 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. + */ + +package org.apache.druid.query.groupby.epinephelinae; + +import it.unimi.dsi.fastutil.Hash; +import it.unimi.dsi.fastutil.objects.Object2IntMap; +import it.unimi.dsi.fastutil.objects.Object2IntOpenCustomHashMap; +import org.apache.datasketches.memory.Memory; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.parsers.CloseableIterator; +import org.apache.druid.query.aggregation.AggregatorAdapters; +import org.apache.druid.query.groupby.epinephelinae.collection.MemoryPointer; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.util.Arrays; +import java.util.Iterator; + +/** + * On-heap {@link VectorGrouper} that grows aggregator state on demand, up to maximum limit of 2GB. + * + * Vectorized analogue of {@link org.apache.druid.query.topn.BaseTopNAlgorithm}'s + * {@code runWithCardinalityUnknown} path: used when dimension cardinality is unknown (numeric columns, + * non-dict-encoded string virtual columns) or when a dict-encoded string column's cardinality exceeds + * the processing buffer. Memory footprint is on-heap and grows with the distinct-key count — matching + * the non-vectorized path's memory profile for the same queries. + */ +public class HeapVectorGrouper implements VectorGrouper Review Comment: This seems kind of similar to `HashVectorGrouper`, used by `groupBy`. The difference I can see is that this one uses heap buffers rather than the processing buffer. It'd be nice to share code and it'd be nice to use the processing buffer here if possible. Other than that, I'm also concerned that there are some aggregators which tend to take up much more space for the `BufferAggregator` / `VectorAggregator` form than for the `Aggregator` form for small numbers of aggregated data points (certain sketches, especially theta sketches but also sometimes hll). With `groupBy` we spill these and then the spill files tend to be very small. The problem is described in #19357 and #19439. So, overflowing into the heap when the processing buffer is full, but continuing to use `VectorAggregator` in-heap, can cause large amounts of heap to be used. I wonder if this would work well: - Start `HashVectorGrouper` (the one from `groupBy`) in the processing buffer - If aggregation completes with `HashVectorGrouper`, emit that - If aggregation stops due to the processing buffer being full, "spill" to heap and then continue aggregating in the processing buffer. - If the total in-heap "spill" memory exceeds some threshold then fail the query. - Once done, merge what's in the buffer along with any "spills" into the final result. -- 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]
