sunchao commented on PR #25434: URL: https://github.com/apache/datafusion/pull/25434#issuecomment-5753683130
Addressed in commit [048610951](https://github.com/apache/datafusion/commit/048610951fbcf5cd7aa566bfd288f5485fd10109), including the requested `mid_20k` and `mid_100k` benchmark cases. The slowdown came from allocating a table for all one million rows, then moving its entries into a smaller table. At the first growth, the builder now samples up to 2,048 positions across the buffered string/binary column and uses that only to choose capacity. Every row is still inserted normally. Uncertain estimates retain full preallocation; underestimated estimates can grow again. Smaller sampled allocations reserve their final compaction workspace upfront. The insertion loop also uses a generic internal iterator while preserving the public boxed API. Against exact upstream base `62f039f0`, both default-release/LTO passes improved all seven benchmark cases. The 20k case went from **32.52 → 13.03 ms** and **64.11 → 13.46 ms**; the 100k case went from **37.25 → 29.33 ms** and **74.63 → 28.79 ms**. These are separate paired runs, not pooled averages: both binaries showed fast/slow timing modes. Runs used the submitted benchmark file, separate target directories, CPU 0, one Tokio worker, 30 samples, 2 s warmup, 5 s measurement, and reversed process order. Extra quiet runs confirmed the unique-key and input-order controls. A separate non-LTO comparison also improved all seven cases and reproduced the old PR head's 20k/100k slowdown. Seven additional stress cases also improved in both paired runs: the initial-table size boundary, skewed inputs with unique rows at either end, and grouped 100k keys. No timing regression reproduced in the final PR benchmark runs or these stress cases. The new tests also cover two failures found during development: a tight-memory LeftAnti query that needs a visited-row bitmap, and an underestimated sample whose denied full-growth attempt previously retained an oversized table. Both regressions pass and verify reservation cleanup; separate focused tests validate row chains and FIFO order. All 19 focused tests pass normally and with forced hash collisions. Extended workspace tests, all 521 SQL logic files, core/CLI tests, strict Clippy, and the full repository lint suite pass locally. The unique-key `build_mem_used` counter still includes charges missing from the base: its existing 4 MB row-index vector and 64 KiB scratch. Growth also temporarily holds a 278,544-byte initial table beside the replacement. Retained unique-key index storage is unchanged; the counter measures reservations, not RSS. The PR description records this distinction and the identical temporary dependency lock used for local comparisons. All GitHub checks for `048610951` are complete: 38 successful, 3 optional skips, no failures. The [Rust CI run](https://github.com/apache/datafusion/actions/runs/35544880552) includes the published lockfile. -- 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]
