malinjawi opened a new pull request, #12738: URL: https://github.com/apache/gluten/pull/12738
Closes #12504. Stacked on #12592 — review that first, this branch contains it. Implements the approach sketched in [this comment](https://github.com/apache/gluten/issues/12504#issuecomment-5035220409), all native, no new JNI and no Scala changes. ### The problem The build side of a BHJ never reaches the native operator as data. Its hash table is built once per executor, or on the driver and broadcast in serialized form, and handed to the join through a process local cache keyed by a hash table id, so the build side input iterator yields no rows. A standalone benchmark process has no such cache, so even with the crashes in #12592 fixed it replays the join against an empty build side and reports timings for a join that produces no output. ### Dumping After converting the plan, `VeloxRuntime` walks it for `HashJoinNode`s that resolved to a cached table, serializes each with the existing `HashTableSerializer`, and writes `hashtable_{stageId}_{partitionId}_{vId}_{cacheKey}.bin`. Resolution goes through `getJoin()`, the same way the converter did a moment earlier, rather than reaching into `HashTableCache` directly — `HashTableCache::get()` is not a read-only accessor, it reserves an entry and registers a query release callback. A failure here is logged and skipped rather than thrown: dumping runs inside a real task and must not fail the query it is attached to. ### Replaying `GenericBenchmark` gains `--hash_table`, which deserializes each file and registers it under its cache key, before any plan is converted. Two registrations, for two different reasons: - `HashTableCache::add` is what lets `HashBuild` take its cached path. An externally added entry is `buildComplete`, so `setupCachedHashTable()` calls `noMoreInput()` and never reads the build side — which is what makes the empty build side input harmless rather than something to work around. - `registerLocalHashTable` is what makes the plan converter choose that path at all, since it probes `getJoin()` and only sets `useHashTableCache` if a table already exists. `getJoin()` now resolves against those process local tables when no JVM is attached, and still reports a miss when there are none. An executor is unaffected: there the JVM side `VeloxBroadcastBuildSideCache` answers first and remains the only authority. Loading happens once at startup rather than per iteration, which is also what keeps `HashTableCache::add` from rejecting a duplicate key on the second benchmark iteration. An externally added entry has no query release callback, so it survives across iterations. ### File format `ignoreNullKeys` and `joinHasNullKeys` travel with the payload because deserializing needs them and neither can be recovered from it: the first selects the `HashTable` specialization, the second is build side state the join semantics depend on. Transposing them would deserialize into the wrong specialization rather than fail outright, so they are covered by their own test. The format lives in one place because the dumping and replaying sides are different binaries. ### Testing `hash_table_dump_file_test` covers the format contract: round trip, each flag independently, empty payload, foreign content, truncation at every offset, and that a cache key cannot escape the dump directory. Compiled and linked locally against velox `dft-2026_07_16`, including `generic_benchmark`. I could not run the tests locally — every Gluten velox test on this macOS build aborts in `folly::f14::rehashImpl` before `main`, including ones this branch does not touch — so runtime verification comes from CI. **Not verified end to end.** No one has yet dumped a BHJ stage from a real query and replayed it with `--hash_table`. @FelixYBW, you have the Q67 setup — this is the piece worth confirming, and I would rather hear it does not work than merge on inference. ### Follow ups, unchanged from #12592 `--data` still binds files to iterator indexes by position while the index is right there in the filename, and plan conversion still calls into JNI, which is why a no-JVM process needed a guard in the first place. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
