RFR: 8309408: Thread.sleep cleanup

[Chris Plummer]

On Mon, 5 Jun 2023 15:10:18 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/ThreadController.java line 660:
>> 
>>> 658:         expectedMethods.add(Thread.class.getName() + ".sleep");
>>> 659:         expectedMethods.add(Thread.class.getName() + ".sleepNanos");
>>> 660:         expectedMethods.add(Thread.class.getName() + ".sleepNanos0");
>> 
>> I'm surprised this test doesn't list `beforeSleep` and `afterSleep`.
>
>> There is one potential, pre-existing, test omission noted below.
>> I'm surprised this test doesn't list `beforeSleep` and `afterSleep`.
> 
> The monitoring/stress/thread tests will fail if they observe an unexpected method name in the stack trace. I don't think it can happen because the tests poll the thread state and for SleepingThread, it will sample the stack trace when the thread state is timed-wait. The beforeSleep/afterSleep methods won't in the stack trace when sleeping. It would be harmless to add them in that they aren't going to cause these tests to fail but might help with any further changes.

The following commit in loom heavily modified this file with a lot of added expected methods. There are other related tests with similar changes. I'm not so sure I understand the need for so many additions, and also why expectedLength is so out of sync with the number of added method. I don't believe this commit was reviewed individually, but was just part of the overall loom review when merge into jdk. Perhaps it should be revisited.

https://github.com/openjdk/loom/commit/26e66bc1a6a0dd735c8138a696809caba3e82b26

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14303#discussion_r1218539693