perf: replace arrow concat_batches by concat_batches_owned - #23598
perf: replace arrow concat_batches by concat_batches_owned#23598huymq1710 wants to merge 2 commits into
Conversation
|
Since #23032 remove |
Thank you. I think the tradeoff between this PR and #23032 is:
Further reducing memory usage with this PR’s approach may be challenging. I did a quick experiment before, and the measured RSS was not as low as 0.5×; if I remember correctly, my result was similar to yours. One possible reason is that the memory allocator caches released memory for future reuse. Therefore, to pursue this PR’s approach, we may need to better understand and tune the memory allocator—or possibly even OS-level behavior. If anyone knows how to do it, this PR's approach would be better (no extra complexity will be introduced to operators); otherwise I plan to help with #23032 later. |
Which issue does this PR close?
concat_batchesin joins #23076Rationale for this change
To avoid 2x memory amplification from concat_batches in joins
AS-IS
/usr/bin/time -l datafusion-cli -f /Users/qmac/Scripts/hj_mem.sql 100000001 row(s) fetched. (First 40 displayed. Use --maxrows to adjust) Elapsed 0.512 seconds. 0.54 real 0.43 user 0.11 sys 2032107520 maximum resident set size <--- 2GBTO-BE
/usr/bin/time -l datafusion-cli -f /Users/qmac/Scripts/hj_mem.sql 100000001 row(s) fetched. (First 40 displayed. Use --maxrows to adjust) Elapsed 0.498 seconds. 0.54 real 0.43 user 0.11 sys 1732083712 maximum resident set size <--- 1.6GBWhat changes are included in this PR?
Add
concat_batches_owned, which similar with https://docs.rs/arrow/latest/arrow/compute/struct.BatchCoalescer.htmlAre these changes tested?
Yes
Are there any user-facing changes?
No