On Thu, 26 May 2022 20:52:43 GMT, liach <d...@openjdk.java.net> wrote:
>> Simplify calls `Class.forName(String, boolean, ClassLoader)` instead of
>> `Class.forName(String)`. `make test
>> TEST="jtreg:test/jdk/java/lang/reflect/Proxy"` passes, with the new
>> `LazyInitializationTest` failing the eager initialization check on the
>> baseline and passing with this patch.
>>
>> On a side note, this might reduce the number of methods that can be encoded
>> in a proxy due to code attribute size restrictions; we probably would
>> address that in another issue, as we never mandated a count of methods that
>> the proxy must be able to implement.
>>
>> Mandy, would you mind review this?
>
> liach has updated the pull request with a new target base due to a merge or a
> rebase. The incremental webrev excludes the unrelated changes brought in by
> the merge/rebase. The pull request contains eight additional commits since
> the last revision:
>
> - Merge branch 'master' into proxy-class-forname
> - Move the try catch block as it doesn't throw checked exceptions
> - remove unused field
> - whitespace
> - Copyright year
> - typo
> - 8285401: Proxy class initializer should use 3-arg `Class.forName` to avoid
> unnecessary class initialization
> - Test for eager initialization
Thanks for doing this. It looks okay. JDK-7194006, having a variant of
`Class::forName` that loads the named class with the class loader of the
current class, would simplify this. This can be updated when such an API is
defined in the future.
test/jdk/java/lang/reflect/Proxy/LazyInitializationTest.java line 53:
> 51: new Class<?>[]{ Intf.class },
> 52: (proxy, method, args) -> null);
> 53: Assert.assertFalse(initialized, "parameter type initialized
> eagerly");
This expects "parameter type initialized eagerly" to be false. This may cause
confusion to the reader. Maybe just simply "initialized expected: false" and a
comment would help too. Similarly for line 56.
test/jdk/java/lang/reflect/Proxy/LazyInitializationTest.java line 53:
> 51: new Class<?>[]{ Intf.class },
> 52: (proxy, method, args) -> null);
> 53: Assert.assertFalse(initialized, "parameter type initialized
> eagerly");
This expects "parameter type initialized eagerly" to be false. This may cause
confusion to the reader. Maybe just simply "initialized expected: false" and a
comment would help too. Similarly for line 56.
-------------
Marked as reviewed by mchung (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8800
