DanielLeens commented on PR #12418: URL: https://github.com/apache/seatunnel/pull/12418#issuecomment-5805502050
Thanks @Vivek1106-04, this answers the piece I was waiting on. The boundary you describe -- `@Setup(Level.Invocation)` parks/spins up to `expected` untimed, and the timed `@Benchmark` region only starts there and ends when `pendingCounter` flips -- means the measured value really is scheduling delay plus the trigger's own decision logic up to executor submission, not delay-plus-wait-for-the-next-trigger. That's the boundary this benchmark needs to be trustworthy, so I'm satisfied on that front. On the SampleTime-vs-AverageTime call: I'd lean toward (a) too (teach `save_jmh_result.py` to read `rawDataHistogram` so CV keeps meaning "variation between iterations") since losing the tail percentiles on a scheduler benchmark seems like the wrong trade-off, but I don't think it's blocking either way -- (b) as a stopgap with (a) as a fast follow-up is fine if you'd rather keep the tooling change out of this PR's description. Appreciate you committing to comments at both read sites (the two reflected fields, and the fake's two-message assumption) explaining what engine behavior each depends on and that the fixture fails loudly at setup if it's missing -- that was the main thing I wanted nailed down given how easy this would be to silently break on an unrelated engine-internals change. Once that's pushed alongside the CV fix (or the AverageTime fallback), I'll do the final pass on the fixture. -- 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]
