> On 24 Jun 2020, at 07:03, David Holmes <david.hol...@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.lev...@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.java Tue Jun
23 10:46:39 2020 +0100
+++ b/src/java.base/share/classes/java/io/ObjectInputStream.java Wed 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);
