On Mon, 5 May 2025 08:16:31 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> No env to test
>
> src/java.base/share/classes/jdk/internal/util/OperatingSystem.java line 139:
>
>> 137: */
>> 138: private static OperatingSystem initOS() {
>> 139: // Called lazily, valueOf has overhead
>
> Hello Chen, in context of this bug fix, what kind of overhead does
> `valueOf()` have?
> The (pre-existing) comment on this `initOS()` method sets an expectation that
> it will be called from the static initializer of this `OperatingSystem` class
> and thus it expects an `ExceptionInInitializerError` to be thrown by the
> static initiliazer, if the operating system name isn't recognized. Any access
> to `OperatingSystem` class would then have resulted in a
> `NoClassDefFoundError`. With this proposed change, the callers of
> `OperatingSystem.current()` would now start seeing an
> `IllegalArgumentException` if for any reason the operating system name isn't
> recognized.
Enum.valueOf -> Class.enumConstantDirectory -> Class.getEnumConstantsShared ->
Method.invoke -> MethodHandleAccessorFactory.makeSpecializedTarget(isStatic =
true) -> MethodHandles.dropArguments -> LambdaForm.editor -> bytecode
generation and loading because this currently cannot be pregenerated by CDS
archive.
If this class is broken, this would probably already surface at build time
because this is used by jlink; otherwise it would have surfaced in Process
tests. I don't think ensuring EIIE vs IAE is worth a test here.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2073707472
