Re: RFR [15] 8248233: Avoid superfluous Class::isRecord invocations during deserialization

[Peter Levart]

On 6/24/20 6:53 PM, Chris Hegarty wrote:


On 24 Jun 2020, at 17:15, Claes Redestad > wrote:


...
It seems ObjectInputStream#isRecord(Class) is now unused. No need for
a new webrev if you choose to remove it.


Good catch, now removed.

On 24 Jun 2020, at 17:25, Peter Levart > wrote:


Hi Chris,

The patch looks good.

Before the patch it made sense to have if (cl != null) in line 750 in 
ObjectStreamClass, but now nothing in the if block depends on cl, so 
you could use if (osc != null) instead. It is true that:


(cl != null) == (osc != null)

always holds there, but reading the code is easier that way, don't 
you think? Maybe you could even consider merging the content of this 
if block into the similar if block that starts in line 771?



Yes, good idea. Done.

Updated webrev:
https://cr.openjdk.java.net/~chegar/8248233/webrev.01

-Chris.


This looks good.


Now next thing to do perhaps is to find out why deserialization of 
classical classes is slower than deserialization of records ;-)



Regards, Peter

Re: RFR [15] 8248233: Avoid superfluous Class::isRecord invocations during deserialization

[Chris Hegarty]

> On 24 Jun 2020, at 17:15, Claes Redestad  wrote:
> 
> ...
> It seems ObjectInputStream#isRecord(Class) is now unused. No need for
> a new webrev if you choose to remove it.

Good catch, now removed.

> On 24 Jun 2020, at 17:25, Peter Levart  wrote:
> 
> Hi Chris,
> 
> The patch looks good.
> 
> Before the patch it made sense to have if (cl != null) in line 750 in 
> ObjectStreamClass, but now nothing in the if block depends on cl, so you 
> could use if (osc != null) instead. It is true that:
> 
> (cl != null) == (osc != null) 
> 
> always holds there, but reading the code is easier that way, don't you think? 
> Maybe you could even consider merging the content of this if block into the 
> similar if block that starts in line 771?
> 
Yes, good idea. Done.

Updated webrev:
  https://cr.openjdk.java.net/~chegar/8248233/webrev.01

-Chris.

Re: RFR [15] 8248233: Avoid superfluous Class::isRecord invocations during deserialization

[Peter Levart]

Hi Chris,


The patch looks good.


Before the patch it made sense to have if (cl != null) in line 750 in 
ObjectStreamClass, but now nothing in the if block depends on cl, so you 
could use if (osc != null) instead. It is true that:



(cl != null) == (osc != null)


always holds there, but reading the code is easier that way, don't you 
think? Maybe you could even consider merging the content of this if 
block into the similar if block that starts in line 771?



Regards, Peter


On 6/24/20 5:46 PM, Chris Hegarty wrote:

A micro-optimisation noticed when working on JDK-8247532.

For further details see:
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-June/067446.html

Webrev:
https://cr.openjdk.java.net/~chegar/8248233/webrev.00/

before:
RecordSerializationBench.deserializeClasses10avgt 10 13.874 ±1.445us/op
 RecordSerializationBench.deserializeClasses       100  avgt   10   
57.839 ±  3.944  us/op
 RecordSerializationBench.deserializeClasses     1000  avgt   10  
515.483 ± 57.275  us/op
 RecordSerializationBench.deserializeRecords       10  avgt   10   
13.563 ±  0.459  us/op
 RecordSerializationBench.deserializeRecords       100  avgt   10   
61.704 ±  2.481  us/op
 RecordSerializationBench.deserializeRecords     1000  avgt   10  
518.671 ± 19.147  us/op

after:
RecordSerializationBench.deserializeClasses10avgt 10 16.021 ±9.091us/op
 RecordSerializationBench.deserializeClasses       100  avgt   10   
58.550 ±  2.164  us/op
 RecordSerializationBench.deserializeClasses     1000  avgt   10  
524.930 ± 49.663  us/op
 RecordSerializationBench.deserializeRecords       10  avgt   10   
12.567 ±  0.711  us/op
 RecordSerializationBench.deserializeRecords       100  avgt   10   
50.235 ±  1.977  us/op
 RecordSerializationBench.deserializeRecords     1000  avgt   10  
421.557 ± 17.348  us/op


-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-8247532

Re: RFR [15] 8248233: Avoid superfluous Class::isRecord invocations during deserialization

[Claes Redestad]

Hi Chris,

On 2020-06-24 17:46, Chris Hegarty wrote:

A micro-optimisation noticed when working on JDK-8247532.

For further details see:
   https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-June/067446.html

Webrev:
   https://cr.openjdk.java.net/~chegar/8248233/webrev.00/


This looks good.

It seems ObjectInputStream#isRecord(Class) is now unused. No need for
a new webrev if you choose to remove it.

/Claes



before:
  RecordSerializationBench.deserializeClasses10  avgt   10   13.874 ±  
1.445  us/op
  RecordSerializationBench.deserializeClasses   100  avgt   10   57.839 ±  
3.944  us/op
  RecordSerializationBench.deserializeClasses  1000  avgt   10  515.483 ± 
57.275  us/op
  RecordSerializationBench.deserializeRecords10  avgt   10   13.563 ±  
0.459  us/op
  RecordSerializationBench.deserializeRecords   100  avgt   10   61.704 ±  
2.481  us/op
  RecordSerializationBench.deserializeRecords  1000  avgt   10  518.671 ± 
19.147  us/op
after:
  RecordSerializationBench.deserializeClasses10  avgt   10   16.021 ±  
9.091  us/op
  RecordSerializationBench.deserializeClasses   100  avgt   10   58.550 ±  
2.164  us/op
  RecordSerializationBench.deserializeClasses  1000  avgt   10  524.930 ± 
49.663  us/op
  RecordSerializationBench.deserializeRecords10  avgt   10   12.567 ±  
0.711  us/op
  RecordSerializationBench.deserializeRecords   100  avgt   10   50.235 ±  
1.977  us/op
  RecordSerializationBench.deserializeRecords  1000  avgt   10  421.557 ± 
17.348  us/op

-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-8247532