david-mollitor-db commented on PR #58803: URL: https://github.com/apache/spark/pull/58803#issuecomment-5779569512
Closing this, with apologies for the churn — and thanks @uros-b, @dongjoon-hyun, @viirya for the time on it. I proposed this on the basis of a local allocation measurement that I'd inadvertently taken with escape analysis disabled (`-XX:-DoEscapeAnalysis`). Re-running a proper same-machine A/B tells the real story: - **EA on** (the default — and what `parseTimestampString` gets on any hot path once C2 compiles it): the original `tz.map(...)` and the pattern-match form allocate identically (332 B/op in my run). The JIT scalar-replaces the capturing closure entirely, so this change removes nothing. - **EA off**: the closure reappears as exactly 16 B/op — real at the bytecode level, but only when EA can't fire. So the closure allocation only exists before C2 kicks in (interpreted / early tiers); once the method is hot, escape analysis already eliminates it. That pre-C2 window isn't worth a change here. @dongjoon-hyun — your request for real benchmark evidence was the right call; the honest evidence is that there's no measurable benefit. Sorry for the noise. Closing. -- 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]
