On Wed, 19 Nov 2025 04:55:05 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Anton Artemov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8366659: Added bool arg to notify_contended_enter > > Thank you for refactoring the test into several tests which share a common > part. It is nice! > May I ask you about more refactoring? I'll inline my comments with the > refactoring suggestions. > @sspitsyn the proposed changes do have an effect on the order of jvmti events > posted. For instance, in the timed-out case: > > Master branch, events posted: wait -> waited -> contended enter -> contended > entered This PR: events posted: wait -> contended enter -> contended entered > -> waited > > I have checked the specs, and it is not really clear: is (re)entering the OM > considered to be a part of `wait` or not? @sspitsyn could you please comment on the order of events and possible issues we can get if the order is changed? > test/hotspot/jtreg/serviceability/jvmti/SuspendWithObjectMonitorWait/SuspendWithObjectMonitorWait1.java > line 218: > >> 216: >> 217: return 0; >> 218: } > > Nit: Launch of `Waiter` and `Resumer`, some testing steps and test shutdown > are identical for all 3 tests. So, they can be factored out into separate > functions located in the base file. Thanks, I have applied a bit more refactoring, trying to move common stuff into the base test class. ------------- PR Comment: https://git.openjdk.org/jdk/pull/27040#issuecomment-3553429559 PR Review Comment: https://git.openjdk.org/jdk/pull/27040#discussion_r2542591879
