RFR: 8370315: [IR-Framework] Allow scenarios to be run in parallel [v3]

[Damon Fenacci]

On Mon, 3 Nov 2025 08:07:08 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> Now the only question remaining is which tests would already benefit from using the parallel version. I guess we can investigate that separately.

Let me file an RFE for that.

> test/hotspot/jtreg/compiler/lib/ir_framework/TestFramework.java line 456:
> 
>> 454:             }
>> 455:         } else {
>> 456:             startWithScenarios(!FORCE_SEQUENTIAL_SCENARIOS && parallel);
> 
> Maybe we can already handle `FORCE_SEQUENTIAL_SCENARIOS` directly in `startParallel()`. Then `parallel` really means parallel. You could also add an additional API comment for `startParallel()` that we can force disable it with the corresponding property flag.

Good idea. Changed.

> test/hotspot/jtreg/compiler/lib/ir_framework/TestFramework.java line 772:
> 
>> 770:                         System.out.println(output);
>> 771:                     }
>> 772:                 }
> 
> This will probably also not be sorted by scenario index? Could we also just gather it here and then dump it after the stream? Maybe we can put `output` into `Outcome` as well as the exceptions by using a `ConcurrentSkipListMap<Scenario, Outcome>` map in the parallel case or a normal `TreeMap` in the non-parallel case.

The idea was to print the output as soon as one process finishes, so that we can follow the progress a bit better (and if there is an TestFormatException we have it printed up to the exception, although this could be done later as well). Of course then it is not sorted...
That said, the output would be cleaner and more readable if we printed it in order as you suggest. Maybe we could even interleave output and exceptions for the same scenario. What do you think? (though I just noticed we throw another `TestRunException` after printing the exceptions)

> test/hotspot/jtreg/compiler/lib/ir_framework/TestFramework.java line 787:
> 
>> 785:         outcomes.stream()
>> 786:                 .filter(o -> o.other() != null)
>> 787:                 .forEach(o -> exceptionMap.put(o.scenario(), o.other()));
> 
> You could use a `ConcurrentSkipListMap` in the parallel case instead of a tree map. This would allow us to modify the map in parallel in the stream processing and simplify the code. Moreover, it will be sorted by scenario index which I'm not sure is currently the case?

It should be (and I wanted to keep as much as possible as it was) but it's much nicer with `ConcurrentSkipListMap`. And there is no need to return any outcome anymore.

> test/hotspot/jtreg/compiler/lib/ir_framework/TestFramework.java line 927:
> 
>> 925:         if (testVMProcess == null) {
>> 926:             throw new TestFrameworkException("TestVMProcess is null");
>> 927:         }
> 
> You can use this utility method instead:
> Suggestion:
> 
>         TestFramework.check(testVMProcess != null, "TestVMProcess must not be null");

Fixed.

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

PR Comment: https://git.openjdk.org/jdk/pull/28065#issuecomment-3492041441
PR Review Comment: https://git.openjdk.org/jdk/pull/28065#discussion_r2495170981
PR Review Comment: https://git.openjdk.org/jdk/pull/28065#discussion_r2495168517
PR Review Comment: https://git.openjdk.org/jdk/pull/28065#discussion_r2495168073
PR Review Comment: https://git.openjdk.org/jdk/pull/28065#discussion_r2495168834