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

Peter Levart peter.levart at gmail.com
Wed Jun 24 09:16:40 UTC 2020


Hi,


I wonder why "isRecord" was not encoded in class modifier bits, like 
"isEnum" was for example. Are all 32 bits already taken? The isEnum() 
does not have the performance problem since getModifiers() native method 
is intrinsified.


Regards, Peter


On 6/24/20 10:20 AM, Chris Hegarty wrote:
>
>
>> On 24 Jun 2020, at 07:03, David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> 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 
>>>> <mailto: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
>
> Yeah, that seems reasonable. The extra bloat should be given due 
> consideration.
>
> I’ve not yet counted how many of these isThis and isThat methods that 
> there are, but I suspect that there are enough that could warrant 
> their state being encoded into a single int or long value on j.l.Class 
> (that could be set lazily by the VM). This would setup a convenient 
> and reasonably efficient location to add future pieces of cached 
> state, like isSealed.
>
>> - 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.
>
> It is relatively inexpensive to ask a non-record class if it is a 
> record, but the converse is not the case.
>
> Java Serialization can probably “workaround” this, since it already 
> has a level of local-class cache state, so we can leverage that [*], 
> which is probably the right thing to do for Java Serialization anyway, 
> but I still think that there is a general tractable problem here.
>
> -Chris.
>
> [*]
> diff -r 3a9521647349 
> src/java.base/share/classes/java/io/ObjectInputStream.java
> --- a/src/java.base/share/classes/java/io/ObjectInputStream.javaTue 
> Jun 23 10:46:39 2020 +0100
> +++ b/src/java.base/share/classes/java/io/ObjectInputStream.javaWed 
> Jun 24 09:07:07 2020 +0100
> @@ -2182,7 +2182,7 @@
> handles.markException(passHandle, resolveEx);
>          }
>
>
> -        final boolean isRecord = cl != null && isRecord(cl);
> +        final boolean isRecord = desc.isRecord();
>          if (isRecord) {
>              assert obj == null;
>              obj = readRecord(desc);
>
>


More information about the core-libs-dev mailing list