RFR 9: 8164908: ReflectionFactory support for IIOP and custom serialization
peter.levart at gmail.com
Fri Oct 21 07:08:11 UTC 2016
I don't know whether the performance of invoking
sun.reflect.ReflactionFactory methods matters very much, but you could
do a nit to help JIT optimize their invocation better. As both
jdk.internal.reflect.ReflectionFactory and the
sun.reflect.ReflectionFactory are singletons, you could use the strategy
of sun.misc.Unsafe which references the delegate instance
(theInternalUnsafe) off a static final field instead of instance final
field. JIT can constant-fold such reference then.
Otherwise I think it is a good strategy to give frameworks limited
access to selected private methods targeted for a special purpose -
serialization in this case, instead of opening the doors for generic
reflection (via setAccessible). It just feels a little strange that this
is new API in an "unsupported" module which destiny is to eventually
"fade-away" once required functionality appears in a supported way. What
is the message?
On 10/20/2016 07:57 PM, Roger Riggs wrote:
> Thanks for the comments..
> Webrev's updated in place with comments so far. (Including David's
> and Amy's)
> On 10/20/2016 8:21 AM, Alan Bateman wrote:
>> On 19/10/2016 20:59, Roger Riggs wrote:
>>> The support in sun.reflect.ReflectionFactory for custom
>>> serialization, such as IIOP input
>>> and output streams, is being expanded beyond the necessary
>>> constructor of a serializable
>>> class to include access to the private methods readObject,
>>> writeObject, readResolve,
>>> writeReplace, etc.
>>> The IIOP implementation is updated to use a combination of
>>> ReflectionFactory and
>>> Unsafe to serialize and deserialize objects and no longer rely on
>>> Tests are included for ReflectionFactory and the affected IIOP classes.
>>> Please review and comment,
>>> jdk repo webrev:
>>> corba repo webrev :
>> I skimmed through the changes.
>> I assume findReadWriteObjectForSerialization should throw
>> InternalError, rather than return null, if IllegalAccessException is
>> thrown (as IAE is not possible here).
>> You've added @since 9 to sun.reflect.ReflectionFactory but that class
>> is not new.
> It was new in 9, but did not previously have @since. Its internal so
> it may not be important.
> Added by 8137058: Clear out all non-Critical APIs from sun.reflect
>> The javadoc for
>> sun.reflect.ReflectionFactory.newConstructorForSerialization doesn't
>> say that it returns null when the Class is not Serializable.
> The current implementation does not check that the argument is
> and there are tests for that. I will update the documentation to not
> specify a Serializable class.
> I would need to track down all the uses to understand that adding the
> check would not break something.
> It also does not check that the requested constructor is for a supertype.
> I think the semantics of newConstructorForSerialization should include
> the search for
> the super-type's noarg constructor instead of IIOP doing that search.
>> For the MH returning methods then I assume the javadoc should say
>> that it returns a direct method handle.
>> The synchronization in the IIOP ObjectStreamClass isn't very clear.
>> Are the invoke*, read*, write* methods all invoked by the same thread
>> that creates the ObjectStreamClass with the lookup method?
> ObjectStreamClass instances are cached and re-used across all threads.
> ObjectStreamClass.init() handles the synchronization of the
> Any thread using the ObjectStreamClass will use the same method handle
> regardless of the
> thread calling the method.
> Thanks, Roger
More information about the core-libs-dev