On Sun, 4 Jun 2023 11:28:33 GMT, Alan Bateman <al...@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. Looks good! Keeping these tests up-to-date is painful, but as you note this is hopefully stabilized now. There is one potential, pre-existing, test omission noted below. Thanks. 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`. ------------- Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14303#pullrequestreview-1461876843 PR Review Comment: https://git.openjdk.org/jdk/pull/14303#discussion_r1217641284
