----- Mail original ----- > De: "Peter Levart" <peter.lev...@gmail.com> > À: "Remi Forax" <fo...@univ-mlv.fr> > Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net>, "Chris Hegarty" > <chris.hega...@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.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.