On Tue, 11 Aug 2026 12:58:32 GMT, Chen Liang <[email protected]> wrote:
>> Claes Redestad has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove excess newline
>
> make/test/BuildMicrobenchmark.gmk line 132:
>
>> 130: SRC := $(MICROBENCHMARK_SRC), \
>> 131: BIN := $(MICROBENCHMARK_CLASSES), \
>> 132: JAVAC_FLAGS := \
>
> I think we might want to extract disabled warnings, javac flags, and java
> flags to another variable, and we can add the `preview` and
> `--enable-preview` flags for the disabled warnings and javac flags.
I agree that there is a lot of code duplication here. Another way of handling
it would be to have the second call to SetupJavaCompilation reference the
values from the first call through the implicit make variables. In that case I
would put the non preview call first, then the preview call would look
something like this:
$(eval $(call SetupJavaCompilation, BUILD_JDK_MICROBENCHMARK_PREVIEW, \
TARGET_RELEASE := $(BUILD_JDK_MICROBENCHMARK_TARGET_RELEASE), \
SMALL_JAVA := $(BUILD_JDK_MICROBENCHMARK_SMALL_JAVA), \
CLASSPATH := $(BUILD_JDK_MICROBENCHMARK_CLASSPATH), \
...
Having explicit variables is perhaps better and less hacky, and don't really
need to cover all the parameters, mostly the flag related stuff.
> make/test/BuildMicrobenchmark.gmk line 157:
>
>> 155: ))
>> 156:
>> 157: $(BUILD_JDK_MICROBENCHMARK): $(JMH_COMPILE_JARS)
>
> Maybe add a line:
>
> $(BUILD_JDK_MICROBENCHMARK_PREVIEW): $(JMH_COMPILE_JARS)
Yes, this is needed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32296#discussion_r3760273822
PR Review Comment: https://git.openjdk.org/jdk/pull/32296#discussion_r3760281243
