RFR: 7903448: JMH: Override the use of compiler hints using a system property [v2]

[Gilles Duboscq]

On Fri, 24 Mar 2023 09:00:36 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Gilles Duboscq has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>> 
>>   Allow forcing the use of compiler hints using a system property
>>   
>>   Using `-Djmh.compilerhints.mode=FORCE_ON` will force the use of compiler
>>   hints. `-Djmh.compilerhints.mode=FORCE_OFF` will prevent the use of compiler
>>   hints. When the system property is not set, or when it is set to `AUTO`,
>>   the current beahviour based on vm name and version will be used.
>
> jmh-core/src/main/java/org/openjdk/jmh/runner/CompilerHints.java line 108:
> 
>> 106:         FORCE_ON("Forced on"),
>> 107:         FORCE_OFF("Forced off"),
>> 108:         AUTO("Automatically selected");
> 
> I am thinking it would be easier if we separate `AUTO_ON` and `AUTO_OFF`? This would eliminate `compilerHintsEnabled`, because you can just use `compilerHintsSelect` for everything, and treat its `null` is uninitialized. Might want to introduce a few helper methods in this enum, like `isAuto()` and `isEnabled()`.

done

> jmh-core/src/main/java/org/openjdk/jmh/runner/CompilerHints.java line 134:
> 
>> 132:      */
>> 133:     private static boolean compilerHintsEnabled() {
>> 134:         if (compilerHintsEnabled != null) {
> 
> Where do we set `compilerHintsEnabled`? I think it needs to be set before every `return` from this method.

done

> jmh-core/src/main/java/org/openjdk/jmh/runner/CompilerHints.java line 235:
> 
>> 233:                 System.err.println("WARNING: Not a HotSpot compiler command compatible VM (\""
>> 234:                         + System.getProperty("java.vm.name") + "-" + System.getProperty("java.version")
>> 235:                         + "\"), compilerHints are disabled.");
> 
> While you are at it, please change "compilerHints" -> "compiler hints".

done

> jmh-core/src/main/java/org/openjdk/jmh/runner/CompilerHints.java line 437:
> 
>> 435:     public static void printHints(PrintStream out) {
>> 436:         out.print("# Compiler hints: " + (compilerHintsEnabled() ? "enabled" : "disabled") + " (" + compilerHintsSelect().desc() + ")");
>> 437:         out.println();
> 
> Let's print this only if `!compilerHintsSelect().isAuto()`. The normal use case is having compiler hints transparently enabled. This is quite a bit different from Blackhole that does have a normal variance in settings...

done

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

PR Review Comment: https://git.openjdk.org/jmh/pull/96#discussion_r1174015912
PR Review Comment: https://git.openjdk.org/jmh/pull/96#discussion_r1174016122
PR Review Comment: https://git.openjdk.org/jmh/pull/96#discussion_r1174015791
PR Review Comment: https://git.openjdk.org/jmh/pull/96#discussion_r1174016021