[VL]Optimize driver side build hash table performance - #12765
Open
JkSelf wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Optimizes Velox driver-side broadcast hash table build/serialization by reducing repeated table walks, shrinking broadcast payloads (excluding raw build-side batches), and improving executor-side reuse of deserialized native hash tables.
Changes:
- Increase serialization chunk size and reuse a per-thread staging buffer for off-heap ↔ stream transfers.
- Memoize Velox hash table serialized size in native code to avoid computing it twice during driver-side broadcast.
- Avoid shipping raw build-side batches in the broadcast payload; add driver-side recovery and executor-side handle memoization/cloning.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gluten-arrow/src/main/java/org/apache/spark/sql/execution/unsafe/UnsafeByteArray.java | Larger chunk size + per-thread shared staging buffer for (de)serialization. |
| cpp/velox/operators/hashjoin/HashTableBuilder.h | Adds cached serialized-size state and reset on table replacement. |
| cpp/velox/jni/JniHashTable.cc | Uses cached serialized-size to avoid repeated full table scans. |
| backends-velox/src/main/scala/org/apache/spark/sql/execution/SerializedHashTableBroadcastRelation.scala | Makes raw build-side relation access explicitly driver-only with clearer failure mode on executors. |
| backends-velox/src/main/scala/org/apache/gluten/execution/VeloxBroadcastBuildSideCache.scala | Tracks serialized hash table in cache entries; adds driver build-side relation lookup and eviction-time release coordination. |
| backends-velox/src/main/scala/org/apache/gluten/execution/SerializedBroadcastHashTable.scala | Excludes raw relation from payload, adds driverRelationId recovery, and memoizes/clones deserialized native handles per JVM. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+196
to
+200
| * `driverSerializedCache` pins the relation on the driver for the lifetime of the broadcast, so | ||
| * this returns a non-null value there even if the broadcast block itself had to be re-read from | ||
| * disk. On executors the cache is always empty and the result is null, which is fine because the | ||
| * raw batches are never consumed there. | ||
| */ |
Comment on lines
+67
to
71
| // `buildSideRelation` is intentionally not written. It holds the raw build side batches, which | ||
| // are only consumed on the driver (DPP key extraction through `transform`, and fallback to | ||
| // vanilla Spark through `deserialized`). Executors read the serialized hash table and never | ||
| // touch the raw batches, so shipping both would roughly double the broadcast payload. | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?