hhhizzz commented on PR #10141:
URL: https://github.com/apache/arrow-rs/pull/10141#issuecomment-5017907694

   I reran the forced-Selectors benchmark on an AMD EPYC 9254 using the exact 
same historical 2-mode benchmark source for both `cc83f7e` and `6c98404` (5 
alternating pairs, CPU pinned). The **regression** still reproduces: L32 is 
`+2.49%` and L40 is `+5.58%`.
   
   I then split L40 into selection clone, reader creation, read-plan build, and 
reader drain. Clone (`-3.06%`), `try_new` (`-2.99%`), and build (`-0.70%`) do 
not regress; the difference is entirely in drain, at roughly `+5%` after 
excluding one noisy round.
   
   However, **this does not appear to be additional work introduced by the 
selector cursor.** The selector hot path is unchanged, and the normalized 
`ParquetRecordBatchReader::next` machine-code stream is identical on both 
revisions (732 instructions). The result is also highly codegen-sensitive: a 
debug/frame-pointer build reverses the result, and with `codegen-units=1` the 
stable gap disappears. Forcing function alignment changes it again. I also 
tried adding `#[inline]` to the affected `read_records` helper; that restored 
the base-like symbol layout but made performance another `2.25%` worse.
   
   My conclusion is that the **residual forced-Selectors difference is a 
default multi-CGU inlining/code-layout artifact** rather than a cursor 
algorithm regression. I would recommend **not** change `SelectorsCursor`, the 
L4–L40 sweep, or the Auto threshold based on this result. The mask-backed Auto 
decision cost from fully enumerating `mask_run_count`, on the other hand, 
remains independently reproducible within the same binary and is still worth 
optimizing.


-- 
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]

Reply via email to