On Mon, 24 Mar 2025 20:22:55 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:
>> Please review following changes, thanks. >> >> - Add `static` to the vm_info for static JDK. The `-version` output now >> contains `static` on static JDK, e.g.: >> >> >> $ static-jdk/bin/java -version >> openjdk version "25-internal" 2025-09-16 >> OpenJDK Runtime Environment (fastdebug build >> 25-internal-adhoc.jianglizhou.jdk) >> OpenJDK 64-Bit Server VM (fastdebug build 25-internal-adhoc.jianglizhou.jdk, >> mixed mode, static, sharing) >> >> $ jdk/bin/java -version >> openjdk version "25-internal" 2025-09-16 >> OpenJDK Runtime Environment (fastdebug build >> 25-internal-adhoc.jianglizhou.jdk) >> OpenJDK 64-Bit Server VM (fastdebug build 25-internal-adhoc.jianglizhou.jdk, >> mixed mode, sharing) >> >> >> Following changes resolve jtreg test failures on static JDK due to >> '-server|-client|-minimal|-zero' flag added by >> `CommandLineOptionTest.getVMTypeOption()` or >> `optionsvalidation.JVMOptionsUtils`. '-server|-client|-minimal|-zero' flags >> are unrecognized on static JDK (please see >> https://bugs.openjdk.org/browse/JDK-8350982). >> >> - Add `jdk.test.lib.Platform.isStatic()`, which checks for `static` in >> `java.vm.info` system property to determine if current test is running on >> static JDK. >> - Change `CommandLineOptionTest` to only call `getVMTypeOption()` on regular >> JDK (`!Platform.isStatic()`). >> - Change `optionsvalidation.JVMOptionsUtils` to only set VMType to >> '-server|-client|-minimal' on regular JDK ( `!Platform.isStatic()`. > > Jiangli Zhou has updated the pull request incrementally with one additional > commit since the last revision: > > Allocate memory for vm_info string using NEW_C_HEAP_ARRAY. I was only going to handle the static part as a special case but you are right that also handling sharing this way makes a lot of sense and cleans up the code. src/hotspot/share/runtime/abstract_vm_version.cpp line 169: > 167: jio_snprintf(vm_info, len, "%s%s%s", mode, > 168: is_vm_statically_linked() ? ", static" : "", > 169: CDSConfig::is_using_archive() ? ", sharing" : ""); I would prefer to see these parts defined as `const char * static_str = ", static"` so we can use `strlen` above rather than having magic numbers. ------------- Changes requested by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24171#pullrequestreview-2712401528 PR Review Comment: https://git.openjdk.org/jdk/pull/24171#discussion_r2011325318
