RFR: 8247532: Records deserialization is slow

[Peter Levart]

Hi Chris,

On 6/16/20 5:15 PM, Chris Hegarty wrote:
> 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.
>
If there is a way for a test to compile several versions of the same 
(record) class and then produce byte streams with it, then we could 
simulate various class-evolution cases (field missing, surplus field, 
change of field type, etc...) without crafting the bytestream by hand. WDYT?


Regards, Peter