RFR: 8247532: Records deserialization is slow

[Paul Sandoz]

Hi Peter,

 199     private Map<ObjectStreamFieldsKey, MethodHandle> deserializationCtrs;

Change to be either ConcurrentMap or ConcurrentHashMap, thereby making it clearer when using.


2679         private static MethodHandle defaultValueExtractorFor(Class<?> pType) {

Can the implementation use MethodHandles,zero? e.g.

  return MethodHandles.dropArguments(MethodHandles.zero(pType), 0, byte[].class, Object[].class);
 
Paul.

> On Jun 16, 2020, at 8:15 AM, Chris Hegarty <chris.hegarty at oracle.com> 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.
>