RFR: 8309408: Thread.sleep cleanup

[Alan Bateman]

On Mon, 5 Jun 2023 07:26:57 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Thread.sleep has had quite a bit of churn recently to support virtual threads, add sleep(Duration), a JFR event, and the change the underlying implementation to support sub-millis precision. I think the changes have settled down now so we can do some small cleanups that came up in PR discussions. The cleanups were kicked down the road as it requires tracking down faraway tests that depend on the stack depth and the names of internal methods. The two cleanups proposed here are:
>> 
>> 1.  Add a private sleepNanos method that creates/commits the JFR event around the sleep, this avoids duplicate code in the 3 sleep methods.
>> 2.  Rename JVM_Sleep to JVM_SleepNanos to make it clear that it takes the sleep time in nanoseconds, esp. when Thread.sleep's parameter is milliseconds.
>
> 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.

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

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