dwsmith1983 commented on PR #5565: URL: https://github.com/apache/datafusion-comet/pull/5565#issuecomment-5487809068
Re-ran the writer benchmark at the current head against the merge base, per @andygrove's request — same input and shapes; description table updated: | shape | base avg | head avg | encode base -> head | |---|---|---|---| | 2,000 parts, zstd-1 | 0.323s | 0.328s | 0.188 -> 0.187s | | 2,000 parts, zstd-6 | 0.743s | 0.746s | 0.606 -> 0.610s | | 10,000 parts, zstd-3 | 0.581s | 0.557s | 0.326 -> 0.297s | The retained-size checks and boundary releases cost nothing measurable at 2,000 partitions, and the 10,000-partition result holds at the head the code actually produces (~4% wall, ~9% encode). @sunchao added `benches/ipc_decode.rs`: frames prebuilt once via ShuffleBlockWriter (Int64 + nullable Utf8), decoded results asserted identical across variants before measuring, four scenarios — repeated small zstd frames, a large-frame control, small frames with a wide-window level-19 frame every 16 (retained workspace passes the cap, context drops and recreates), and a NONE control — each with one reused context vs a fresh context per frame (the per-frame-creation profile of the base path; head-fresh matches a trimmed copy of the bench run on the base tree within noise: 7.289 vs 7.320 ms on the small-frames scenario). | scenario | reused | fresh per frame | |---|---|---| | 64 small zstd-3 frames | 7.285 ms | 7.289 ms | | 1 large zstd-3 frame | 8.945 ms | 8.918 ms | | over-cap recovery | 7.772 ms | 7.779 ms | | 64 small NONE frames | 337 us | 342 us | Honest read: decode reuse is throughput-neutral here — about 56 ns/frame of context setup against ~114 us/frame of decode work — and the over-cap drop/recreate adds nothing measurable, so no regression from the session reset or the operator-owned locking either. The decode-side case for the context API is the bounded retained workspace, not speed. Creation counts are pinned by the test counters and retained-vs-released sizes by the measured table in codec_context.rs; I did not instrument peak RSS beyond sizeof — can add if you would like. -- 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]
