[VL] Add hash table memory usage metric when enabling spark.gluten.velox.buildHashTableOncePerExecutor.enabled#12528
Open
JkSelf wants to merge 1 commit into
Open
[VL] Add hash table memory usage metric when enabling spark.gluten.velox.buildHashTableOncePerExecutor.enabled#12528JkSelf wants to merge 1 commit into
JkSelf wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Spark SQL metric to report retained hash table memory usage for Velox broadcast hash joins when the “build hash table once per executor” path is used, wiring the value from native Velox hash table allocation tracking through JNI into Spark metrics.
Changes:
- Track hash table retained bytes in native (
HashTableBuilder) and expose it via a new JNI/Java API (HashJoinBuilder.getHashTableMemoryUsage). - Populate the memory usage value during hash table build (including parallel-build subtables) and report it into a new Spark
SizeMetric(hashTableMemorySize). - Thread the new metric through broadcast hash join context/plumbing so it’s recorded when building the executor-side cached hash table.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cpp/velox/operators/hashjoin/HashTableBuilder.h | Adds storage/accessors for a cached hash table memory usage value. |
| cpp/velox/jni/VeloxJniWrapper.cc | Captures allocated bytes during native build and adds a JNI entrypoint to query the metric. |
| cpp/velox/jni/JniHashTable.h | Declares hashTableMemoryUsage(...) native helper. |
| cpp/velox/jni/JniHashTable.cc | Implements hashTableMemoryUsage(...) accessor (note: needs fallback for deserialized handles). |
| backends-velox/src/main/scala/org/apache/gluten/execution/VeloxBroadcastBuildSideCache.scala | Updates the new SQL metric when building a cached broadcast hash table. |
| backends-velox/src/main/scala/org/apache/gluten/execution/HashJoinExecTransformer.scala | Threads the new metric through BroadcastHashJoinContext. |
| backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxMetricsApi.scala | Registers hashTableMemorySize as a new Spark size metric. |
| backends-velox/src/main/java/org/apache/gluten/vectorized/HashJoinBuilder.java | Adds the new native method declaration for querying hash table memory usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+183
to
+186
| int64_t hashTableMemoryUsage(std::shared_ptr<HashTableBuilder> builder) { | ||
| VELOX_CHECK_NOT_NULL(builder, "Hash table builder cannot be null"); | ||
| return builder->hashTableMemoryUsage(); | ||
| } |
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?
Add hash table memory usage metric when enabling spark.gluten.velox.buildHashTableOncePerExecutor.enabled.
How was this patch tested?
local verified
Was this patch authored or co-authored using generative AI tooling?