Optimize hash code generation for exchanges by raunaqmorarka · Pull Request #27657 · trinodb/trino

raunaqmorarka · 2025-12-15T20:31:18Z

Description

Optimize InterpretedHashGenerator using byte code generation and batched loops.
This also brings optimised handling of dictionaries to FlatGroupByHash

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## General
* Improve performance of queries with data exchanges or aggregations. ({issue}`27657`)

Copilot

Pull request overview

This PR optimizes hash code generation for data exchanges and aggregations by introducing bytecode generation and batched loop processing through a new NullSafeHashCompiler class. The optimization focuses on improving the performance of hash computation across multiple positions in a single operation rather than computing hashes one position at a time.

Key changes:

Introduced NullSafeHashCompiler and NullSafeHash classes for bytecode-based hash generation with batched operations
Enhanced InterpretedHashGenerator to use batched hash computation with specialized handling for RLE and dictionary blocks
Added batched methods (getBuckets, getPartitions) to BucketFunction and PartitionFunction interfaces with default implementations

Reviewed changes

Copilot reviewed 57 out of 57 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
core/trino-main/src/main/java/io/trino/operator/NullSafeHashCompiler.java	New compiler class that generates bytecode for efficient batched hash computation
core/trino-main/src/main/java/io/trino/operator/NullSafeHash.java	New interface defining batched hash methods for single blocks
core/trino-main/src/main/java/io/trino/operator/InterpretedHashGenerator.java	Refactored to use batched hash operations with optimized RLE and dictionary handling
core/trino-main/src/main/java/io/trino/operator/FlatHashStrategyCompiler.java	Updated to delegate batched hash operations to InterpretedHashGenerator
core/trino-spi/src/main/java/io/trino/spi/connector/BucketFunction.java	Added `getBuckets` method for batch bucket computation
core/trino-main/src/main/java/io/trino/operator/PartitionFunction.java	Added `getPartitions` method for batch partition computation
core/trino-main/src/main/java/io/trino/operator/HashGenerator.java	Added `hash` and `getPartitions` methods for batch operations
core/trino-main/src/main/java/io/trino/operator/output/PagePartitioner.java	Simplified to use batched partition computation, removing dictionary-specific optimization
core/trino-main/src/main/java/io/trino/sql/planner/SystemPartitioningHandle.java	Updated to use NullSafeHashCompiler instead of TypeOperators
plugin/trino-hive/src/main/java/io/trino/plugin/hive/HivePageSink.java	Updated to use batched bucket computation
Various test files	Updated test setup to provide NullSafeHashCompiler instances

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

core/trino-main/src/main/java/io/trino/operator/InterpretedHashGenerator.java

starburstdata-automation · 2025-12-15T20:50:18Z

Started benchmark workflow for this PR with test type = iceberg/sf1000_parquet_part.

Building Trino finished with status: success
Benchmark finished with status: failure
Comparing results to the static baseline values, follow above workflow link for more details/logs.
Status message: Found regressions for:<br/>(presto/tpcds, q09, totalCpuTime, over by 24.6%)
Benchmark Comparison to the closest run from Master: Report

starburstdata-automation · 2025-12-15T20:50:34Z

Started benchmark workflow for this PR with test type = iceberg/sf1000_parquet_unpart.

Building Trino finished with status: success
Benchmark finished with status: success
Comparing results to the static baseline values, follow above workflow link for more details/logs.
Status message: NO Regression found.
Benchmark Comparison to the closest run from Master: Report

raunaqmorarka · 2025-12-16T06:23:22Z

pettyjamesm

Mostly LGTM, some small comments and I do think there are some places where the temporary allocations might be ripe for further improvements.

pettyjamesm · 2025-12-16T14:06:35Z

core/trino-main/src/main/java/io/trino/operator/exchange/PartitioningExchanger.java

        for (int position = 0; position < partitionPage.getPositionCount(); position++) {
-            int partition = partitionFunction.getPartition(partitionPage, position);
-            partitionAssignments[partition].add(position);
+            partitionAssignments[partitions[position]].add(position);


There's probably an improvement to be gained by fusing this loop with partitionAssignments.getPartitions, but we can worry about that as a follow up.

That is better explored as a follow-up, I expect that might need a different method in PartitionFunction

pettyjamesm · 2025-12-16T14:12:16Z

core/trino-main/src/main/java/io/trino/operator/HashGenerator.java

+
+    default void getPartitions(int partitionCount, int positionOffset, Page page, int length, int[] partitions)
+    {
+        long[] hashes = new long[length];


Reusing this array is likely to be beneficial for most operator use cases, since allocating a new instance on each invocation is non-trivial allocation pressure. Having this default implementation seems like a performance hazard

The allocation here is very short-lived and the JVM is pretty good at optimizing for that. Trying to reuse array adds some complexity as it has to be passed down from the calling operator, where it potentially needs to be tracked as a retained memory allocation. Since we didn't observe a problem with this in production for a while, I'm inclined to keep it simple for now and explore reuse as a follow-up.

core/trino-main/src/main/java/io/trino/operator/InterpretedHashGenerator.java

core/trino-main/src/main/java/io/trino/operator/FlatHashStrategyCompiler.java

core/trino-main/src/main/java/io/trino/operator/NullSafeHashCompiler.java

pgandhi999 · 2025-12-16T20:45:49Z

I also got a chance to run the micro benchmark that I had performed for my PR(#27610) on your PR and the results align with what was stated on my PR. Thank you @raunaqmorarka.

Introduce batched implementations for hashing pages for exchange

Use byte code generation to avoid megamorphic call sites in hash code generation

Uses separate loop per type

starburstdata-automation · 2025-12-17T16:01:06Z

Started benchmark workflow for this PR with test type = iceberg/sf1000_parquet_part.

Building Trino finished with status: success
Benchmark finished with status: failure
Comparing results to the static baseline values, follow above workflow link for more details/logs.
Status message: Found regressions for:<br/>(presto/tpcds, q09, totalCpuTime, over by 31.1%)
Benchmark Comparison to the closest run from Master: Report

starburstdata-automation · 2025-12-17T16:01:16Z

Started benchmark workflow for this PR with test type = iceberg/sf1000_parquet_unpart.

Building Trino finished with status: success
Benchmark finished with status: success
Comparing results to the static baseline values, follow above workflow link for more details/logs.
Status message: NO Regression found.
Benchmark Comparison to the closest run from Master: Report

cla-bot bot added the cla-signed label Dec 15, 2025

github-actions bot added the delta-lake Delta Lake connector label Dec 15, 2025

raunaqmorarka requested a review from pettyjamesm December 15, 2025 20:31

github-actions bot added hive Hive connector postgresql PostgreSQL connector labels Dec 15, 2025

raunaqmorarka requested review from Copilot, dain, findepi, pgandhi999 and sopel39 December 15, 2025 20:31

Copilot AI reviewed Dec 15, 2025

View reviewed changes

core/trino-main/src/main/java/io/trino/operator/InterpretedHashGenerator.java Show resolved Hide resolved

raunaqmorarka force-pushed the raunaq/hashing-opt branch from c7c6711 to 0518ffa Compare December 15, 2025 21:53

raunaqmorarka requested review from losipiuk and lukasz-stec December 16, 2025 06:20

raunaqmorarka added the performance label Dec 16, 2025

pettyjamesm reviewed Dec 16, 2025

View reviewed changes

pgandhi999 mentioned this pull request Dec 16, 2025

Vectorize hashing for Exchange and Local Exchange Operators #27610

Closed

Batch io.trino.spi.connector.BucketFunction

45a6449

Introduce batched implementations for hashing pages for exchange

raunaqmorarka force-pushed the raunaq/hashing-opt branch from 0518ffa to ef51fdc Compare December 17, 2025 08:44

findepi approved these changes Dec 17, 2025

View reviewed changes

Compile null safe hash operator

cbe0e0e

Use byte code generation to avoid megamorphic call sites in hash code generation

raunaqmorarka force-pushed the raunaq/hashing-opt branch from ef51fdc to 01014c5 Compare December 17, 2025 09:32

Optimize large dictionary handling in InterpretedHashGenerator#hash

49f9c99

Uses separate loop per type

raunaqmorarka force-pushed the raunaq/hashing-opt branch from 01014c5 to 70c2e76 Compare December 17, 2025 09:41

Use InterpretedHashGenerator in FlatGroupByHash

8009b5c

raunaqmorarka force-pushed the raunaq/hashing-opt branch from 70c2e76 to 8009b5c Compare December 17, 2025 09:43

raunaqmorarka requested a review from pettyjamesm December 17, 2025 09:52

pettyjamesm approved these changes Dec 17, 2025

View reviewed changes

raunaqmorarka merged commit d8c8057 into trinodb:master Dec 17, 2025
102 checks passed

raunaqmorarka deleted the raunaq/hashing-opt branch December 17, 2025 15:26

github-actions bot added this to the 480 milestone Dec 17, 2025

chenjian2664 mentioned this pull request Dec 22, 2025

Add 480 release notes #27719

Merged

Conversation

raunaqmorarka commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

starburstdata-automation commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

starburstdata-automation commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raunaqmorarka commented Dec 16, 2025

Uh oh!

pettyjamesm left a comment

Choose a reason for hiding this comment

Uh oh!

pettyjamesm Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

raunaqmorarka Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

pettyjamesm Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

raunaqmorarka Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pgandhi999 commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

starburstdata-automation commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

starburstdata-automation commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

raunaqmorarka commented Dec 15, 2025 •

edited

Loading

starburstdata-automation commented Dec 15, 2025 •

edited

Loading

starburstdata-automation commented Dec 15, 2025 •

edited

Loading

pgandhi999 commented Dec 16, 2025 •

edited

Loading

starburstdata-automation commented Dec 17, 2025 •

edited

Loading

starburstdata-automation commented Dec 17, 2025 •

edited

Loading