RFR: 8247532: Records deserialization is slow

[forax at univ-mlv.fr]

----- Mail original -----
> De: "Peter Levart" <peter.levart at gmail.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net>, "Chris Hegarty" <chris.hegarty at oracle.com>
> Envoyé: Mercredi 17 Juin 2020 07:37:16
> Objet: Re: RFR: 8247532: Records deserialization is slow

> Hi Remi,
> 
> 
> The keys used are based on the ordered names and types of "fields" in
> the ObjectStreamClass that has been deserialized as part of the object
> stream for the record "type" being deserialized. So for each record type
> (Class<? extends Record>) there can be several distinct
> ObjectStreamClasses deserialized in the same VM that were produced by
> serializing different versions of record types in different VMs or
> ClassLoader(s) and that may vary in the names and/or types and/or order
> of fields. And since the MethodHandle chain of transformations that is
> being used for a particular ObjectStreamClass is dependent on the
> ordered names and types of ObjectStreamField(s) of deserialized
> ObjectStreamClass, the key is based on that too.
> 
> You may ask why not simply add a MethodHandle field to the
> ObjectStreamClass instance that is deserialized and therefore unique?
> Well, such ObjectStreamClass only lasts for one "session" of
> ObjectStream deserialization, so such caching wouldn't be efficient. But
> common parts of that ObjectStreamClass that are only dependent on the
> current local-VM type are cached in a special instance of
> ObjectStreamClass and that's where I put a cache of deserializaiton
> constructors in the form of a Map.

Ok, i see, you want to cache all permutations not only the one corresponding to the current record order.

> 
> 
> Regards, Peter

Rémi

> 
> 
> On 6/17/20 1:06 AM, Remi Forax wrote:
>> Hi Peter,
>> is their a reason to not use a ClassValue<MethodHandle> using the record class
>> as key instead of the more complex keys you are proposing ?
>>
>> Rémi
>>
>> ----- Mail original -----
>>> De: "Chris Hegarty" <chris.hegarty at oracle.com>
>>> À: "Peter Levart" <peter.levart at gmail.com>
>>> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net>
>>> Envoyé: Mardi 16 Juin 2020 17:15:03
>>> Objet: Re: RFR: 8247532: Records deserialization is slow
>>> Hi Peter,
>>>
>>>> On 14 Jun 2020, at 17:28, Peter Levart <peter.levart at gmail.com> wrote:
>>>>
>>>> ...
>>>>
>>>> [2]
>>>> http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.01/
>>>>
>>> I think that this is very good. It’s clever to build a chain of method handles
>>> that can be invoked with the stream field values. This is a nice separation
>>> between the shape of the data and the actual stream data.
>>>
>>> The caching is on a per-stream-field shape basis, which should be consistent in
>>> the common case, but of course is not always guaranteed to be the case, hence
>>> the need for the cache. I think that this should be fine, since the
>>> ObjectStreamClass ( that holds the cache ) is already itself cached as a weak
>>> reference. But I did wonder if the size of this new cache should be limited.
>>> Probably not worth the complexity unless it is an obvious issue.
>>>
>>> All the serailizable records tests pass successfully with your patch. Good. I
>>> did however notice that there is just a single test, DefaultValuesTest, that
>>> exercises different stream shapes for the same class in the stream. Would be
>>> good to expand coverage in this area, but of course some lower-level
>>> test-specific building blocks will be needed help build the specially crafted
>>> byte streams - I can help with this.
>>>
>>> Overall I think that this change is good.
>>>
> >> -Chris.