sunchao commented on PR #25491: URL: https://github.com/apache/datafusion/pull/25491#issuecomment-5744143986
Thanks @comphead, @jayzhan211, and @viirya. I pushed [65f6a1d](https://github.com/apache/datafusion/commit/65f6a1d4284dbb0b572576e7c8a17a8e28e7243e) and replied to the inline threads. The shared row-index accounting bug is now a separate prerequisite, [#25508](https://github.com/apache/datafusion/pull/25508), and this branch includes the same fix. The collector now uses named sizing helpers and ingestion-time totals; `EXPLAIN` identifies attached builds, and the public docs lead with the snapshot and lifetime contracts. The motivation is sharing a retained build and avoiding repeated build work across independent plans. In the companion Comet integration, one producer materializes and hashes the shared build while matching consumers wait; the consumers then probe independently and can run in parallel. It does not serialize all probes. Ordinary joins already use `OnceFut`/`Shared` and `JoinLeftData`, although attaching a prepared build still has setup costs. The new benchmark includes cold preparation before concurrent probes and warm reuse with an explicit retained lease. The large four-consumer cases still favored reuse in reverse-order runs, while small builds remained mixed. An initial +6.0% ordinary general-hash result became +0.8% in a longer reverse-order comparison with overlapping intervals; I am not treating those controls as evidence of a repeatable ordinary-path improvement or regression. The updated description contains the full timing matrix and the ordinary-path before/after controls, including the small-build cases. These are DataFusion execution-API measurements, not Comet end-to-end measurements: decoding, cache coordination, waiter behavior, and actual process CPU/RSS are outside this benchmark. Pool reservations are reported with their scope and are not presented as physical-memory ratios or a universal break-even threshold. I kept the existing construction API with a compiled example and a prominent supplied-stream/snapshot contract. I also kept the concat invariant error: reserving after a copy has already been allocated cannot admit the earlier peak. The mixed-validity regression checks actual Arrow buffer capacity, including the case where a null-containing input creates validity for rows from an input without a null bitmap. Validation now passes locally against the requested Arrow 60 release sources: full workspace Clippy with all targets/features and warnings denied, the extended workspace run (11,995 Rust tests plus 521 SQL test files; eight tests ignored), 550 focused hash-join tests, the serialization regression, and the API doctest. Temporary local dependency-source overrides are documented in the description and are not in the patch. I removed the stale draft/Arrow-blocker text; CI on the new revision is separate from these local results. -- 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]
