On Wed, 12 Mar 2025 11:13:41 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
>> make/modules/java.management/Lib.gmk line 35: >> >>> 33: >>> 34: LIBMANAGEMENT_OPTIMIZATION := HIGH >>> 35: ifeq ($(call isTargetOs, linux)+$(COMPILE_WITH_DEBUG_SYMBOLS), >>> true+true) >> >> On removal of `ifeq ($(call isTargetOs, >> linux)+$(COMPILE_WITH_DEBUG_SYMBOLS), true+true)` >> ..are we saying this is redundant? >> >> It reads like Linux builds with LOW, and this change will change that to >> HIGH ? >> >> I tested existing build and see -O2 in Linux fastdebug and release builds. >> So this ifeq wasn't doing anything? >> >> Windows fastdebug and release I just checked and saw -O1, I'm not sure why >> that is. >> >> We do the same thing in make/modules/jdk.management/Lib.gmk so both these >> management locations should probably be treated the same. >> >> (The same comparison is in make/modules/java.base/lib/CoreLibraries.gmk >> affecting LIBVERIFY_OPTIMIZATION, but no need to expand this change beyond >> the management area.) > > @kevinjwalls >> On removal of `ifeq ($(call isTargetOs, >> linux)+$(COMPILE_WITH_DEBUG_SYMBOLS), true+true)` ..are we saying this is >> redundant? >> >> It reads like Linux builds with LOW, and this change will change that to >> HIGH ? > > No, we are not saying it is redundant. Exactly as you say this PR will change > the value from LOW to HIGH. The assumption is that this special case was > created more than a decade ago, and no ill effect seems to arise when > removing the exception, so it seems to be needed no more. > > This is part of Matthias ongoing effort to simplify and modernize the > optimization levels of native libraries in the JDK. Thanks yes, I could not reconcile the "most likely not needed" statement with the implied "this will change the opt level", so the intent wasn't clear at first. I think we're in sync now. Agreed the opt level should be a good change! ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23966#discussion_r1991262315