On Thu, 4 May 2023 20:17:11 GMT, Eric Caspole <ecasp...@openjdk.org> wrote:
> These micros were developed while investigating JDK-8305670 by myself and
> Sergey Kuksenko. The order of thread creation was important in that bug, so
> there are 2 JMH for creating sleepers before and after the worker threads.
Cursory style reviews.
test/micro/org/openjdk/bench/java/util/concurrent/UnparkBenchSleepersAfter.java
line 90:
> 88:
> 89:
> 90: ExecutorService exec;
Suggestion:
IdleThread[] idleThreads;
ExecutorService exec;
test/micro/org/openjdk/bench/java/util/concurrent/UnparkBenchSleepersAfter.java
line 99:
> 97: for (int i = 0; i < workers; i++) { // warmup exec
> 98: exec.submit(() ->
> 99: {
Suggestion:
exec.submit(() -> {
test/micro/org/openjdk/bench/java/util/concurrent/UnparkBenchSleepersAfter.java
line 113:
> 111: for(int i=0; i < idle_threads.length; i++) {
> 112: new Thread(idle_threads[i] = new IdleThread()).start();
> 113: }
These `idleThreads` are not actually `Thread`-s, they are `Runnable`-s.
Suggestion:
for(int i = 0; i < idles; i++) {
Runnable r = new IdleRunnable();
idleRunnables[i] = r;
new Thread(r).start();
}
test/micro/org/openjdk/bench/java/util/concurrent/UnparkBenchSleepersAfter.java
line 118:
> 116: @TearDown
> 117: public void tearDown() {
> 118: for(IdleThread it : idle_threads) {
Suggestion:
for (IdleThread it : idle_threads) {
test/micro/org/openjdk/bench/java/util/concurrent/UnparkBenchSleepersAfter.java
line 126:
> 124: public static class IdleThread implements Runnable {
> 125: boolean done = false;
> 126: Thread my_thread;
Suggestion:
Thread myThread;
-------------
PR Review: https://git.openjdk.org/jdk/pull/13815#pullrequestreview-1416443702
PR Review Comment: https://git.openjdk.org/jdk/pull/13815#discussion_r1187224644
PR Review Comment: https://git.openjdk.org/jdk/pull/13815#discussion_r1187224912
PR Review Comment: https://git.openjdk.org/jdk/pull/13815#discussion_r1187226283
PR Review Comment: https://git.openjdk.org/jdk/pull/13815#discussion_r1187226379
PR Review Comment: https://git.openjdk.org/jdk/pull/13815#discussion_r1187226532
