RFR: 8247532: Records deserialization is slow

[Remi Forax]

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.