On Mon, 5 Jun 2023 15:10:18 GMT, Alan Bateman <al...@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
