RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

[David Holmes]

Hi Chris,

On 24/06/2020 2:30 am, Chris Hegarty wrote:
> 
>> On 23 Jun 2020, at 14:49, Peter Levart <peter.levart at gmail.com> wrote:
>>
>> ...
>>
>> Ok, I'm going to push this to jdk15.
> 
> Thank you Peter. This is a really nice change.
> 
> As a follow on, and not for JDK 15, I observe that Class::isRecord0 / JVM_IsRecord shows up as consuming a significant amount of time, more than 10%, in some runs of the deserialization benchmark. The isRecord implementation is a native method in the JVM, so relatively expensive to call.
> 
> This shows an opportunity to improve the Class::isRecord implementation with a simple cache of the record-ness of the j.l.Class, as is done selectively with other particular aspects of a class’s state. There are various ways to implement this, but here is just one [*].

There has been reluctance to add more and more fields to Class to cache 
all these new attributes that are being added - but ultimately that is a 
call for core-libs folk to make. The general expectation is/was that the 
need to ask a class if it is a Record (or isSealed etc) would be rare. 
But (de)serialization is the exception for isRecord() as unlike enums a 
simple instanceof test can't be used.

Cheers,
David
-----

> Running the deserialization benchmark with this change [*], gives the following results:
> 
> Benchmark                                    (length)  Mode  Cnt    Score    Error  Units
> RecordSerializationBench.deserializeClasses        10  avgt   10   14.136 ±  0.841  us/op
> RecordSerializationBench.deserializeClasses       100  avgt   10   61.821 ±  1.279  us/op
> RecordSerializationBench.deserializeClasses      1000  avgt   10  519.473 ±  7.950  us/op
> RecordSerializationBench.deserializeRecords        10  avgt   10   13.781 ±  1.917  us/op
> RecordSerializationBench.deserializeRecords       100  avgt   10   54.061 ±  4.188  us/op
> RecordSerializationBench.deserializeRecords      1000  avgt   10  444.538 ± 13.940  us/op
> 
> I think it is worth considering caching the record-ness state of a j.l.Class, as I’m sure it will be widely used in third-party serialization libraries, as well as by Java Serialization.
> 
> -Chris.
> 
> [*]
> diff -r 3a9521647349 src/java.base/share/classes/java/lang/Class.java
> --- a/src/java.base/share/classes/java/lang/Class.java	Tue Jun 23 10:46:39 2020 +0100
> +++ b/src/java.base/share/classes/java/lang/Class.java	Tue Jun 23 17:11:35 2020 +0100
> @@ -3712,9 +3712,17 @@
>       @jdk.internal.PreviewFeature(feature=jdk.internal.PreviewFeature.Feature.RECORDS,
>                                    essentialAPI=false)
>       public boolean isRecord() {
> -        return getSuperclass() == JAVA_LANG_RECORD_CLASS && isRecord0();
> +        if (!isRecordCalled) {
> +            isRecord = getSuperclass() == JAVA_LANG_RECORD_CLASS && isRecord0();
> +            isRecordCalled = true;
> +        }
> +        return isRecord;
>       }
>   
> +    // cached record(ness) status
> +    private transient boolean isRecord;
> +    private transient boolean isRecordCalled;
> +
>       // Fetches the factory for reflective objects
>       private static ReflectionFactory getReflectionFactory() {
>           if (reflectionFactory == null) {
>