uros-b commented on code in PR #56975: URL: https://github.com/apache/spark/pull/56975#discussion_r3524964173
########## sql/core/src/main/java/org/apache/spark/sql/execution/aggregate/RowBasedAggregateHashMap.java: ########## @@ -0,0 +1,117 @@ +/* + * 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.spark.sql.execution.aggregate; + +import org.apache.spark.memory.TaskMemoryManager; +import org.apache.spark.sql.catalyst.InternalRow; +import org.apache.spark.sql.catalyst.expressions.RowBasedKeyValueBatch; +import org.apache.spark.sql.catalyst.expressions.UnsafeProjection; +import org.apache.spark.sql.catalyst.expressions.UnsafeRow; +import org.apache.spark.sql.catalyst.expressions.codegen.UnsafeRowWriter; +import org.apache.spark.sql.types.StructType; +import org.apache.spark.unsafe.KVIterator; +import org.apache.spark.unsafe.Platform; + +/** + * Shared, type-independent machinery for the row-based fast hash map that {@code HashAggregateExec} + * generates as the level-1 cache during hash aggregation (see {@code RowBasedHashMapGenerator}). + * + * The generated subclass supplies only the schema-specific, strongly-typed methods that must be + * codegen'd for speed -- {@code findOrInsert}, {@code equals}, and {@code hash} -- while the + * boilerplate that does not depend on the key/value schema (the batch and bucket bookkeeping, the + * empty-buffer setup, the row iterator, and {@code close}) lives here so it is written and JIT'd + * once per JVM rather than re-emitted into every aggregation stage's generated class. + * + * The hot path is unchanged: the bucket scan, hash, equals, and typed key write still run in the + * generated subclass; the one helper the subclass calls into here ({@link #appendCurrentKey(int)}) + * is {@code final} so HotSpot can inline it. + * + * NOTE: like the generated map it replaces, this does not support nullable keys; the caller falls + * back to the {@code BytesToBytesMap} for those. + */ +public abstract class RowBasedAggregateHashMap implements AutoCloseable { + // NOTE: these field names are part of the contract with the generated subclass -- + // RowBasedHashMapGenerator emits code that references them by name (e.g. `agg_rowWriter`, Review Comment: Nit: `agg_rowWriter` carries a codegen naming prefix in a permanent hand-written class; the name is a load-bearing runtime contract with the generator, so a future rename compiles in Java but breaks Janino only at runtime. A clean name (`rowWriter`) + generator update would eliminate the coupling at near-zero cost. Maintainability nit, not a correctness defect. -- 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]
