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.hega...@oracle.com> > À: "Peter Levart" <peter.lev...@gmail.com> > Cc: "core-libs-dev" <core-libs-dev@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.lev...@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.
