RFR: 8333796: Add missing serialization functionality to sun.reflect.ReflectionFactory [v9]

David M. Lloyd duke at openjdk.org
Wed Nov 13 19:38:56 UTC 2024


On Wed, 13 Nov 2024 17:47:57 GMT, Chen Liang <liach at openjdk.org> wrote:

>> David M. Lloyd has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 37 commits:
>> 
>>  - Merge remote-tracking branch 'upstream-jdk/master' into serialization
>>  - Round out the documentation of the new methods to explain the supported and unsupported cases
>>  - Move `serialPersistentFields` for a degree of method order consistency
>>  - Address review feedback
>>  - Test fixes and finish renaming
>>  - Address review feedback
>>  - Address review comment
>>  - Eliminate cache
>>  - Rework using facilities found in ObjectStreamClass
>>    
>>    This variation has the disadvantage of requiring a couple temporary arrays to be allocated each time the method handles are used.
>>  - More tests
>>  - ... and 27 more: https://git.openjdk.org/jdk/compare/133f8f31...d57c4346
>
> src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 80:
> 
>> 78:     private ReflectionFactory() {
>> 79:         this.langReflectAccess = SharedSecrets.getJavaLangReflectAccess();
>> 80:         this.javaObjectStreamReflectionAccess = SharedSecrets.getJavaObjectStreamReflectionAccess();
> 
> ReflectionFactory is created on very early bootstrap, even before `JavaLangAccess` is ready.  This extra dependency is only used when the unsupported version is used, and this dependency, calling MethodHandle, should have changed the boot class loading/initialization sequence by making MH initialization early.  Can you check with a hello world app and run it with `-Xlog:class+init` to check the initialization sequence?
> 
> I strongly recommend not caching this access object, and just call `getJavaObjectStreamReflectionAccess()` on demand.

I only added the field for consistency with the existing one (which predates my PR), but inlining it is fine with me. I'll make that change now.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19702#discussion_r1841053650


More information about the core-libs-dev mailing list