RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

Brent Christian-2
Hi,

There are a couple opportunities to reduce the memory footprint of
java.lang.StackFrameInfo (the internal implementation of
java.lang.StackWalker.StackFrame):

8153682[1] : StackFrameInfo.declaringClass could be removed
8185925[2] : StackFrameInfo::walker field can be replaced with bitmap to
save footprint

I had a look using jol[3].  Removing only 'walker' helps only under 32-
and 64-bit, but not with compressed oops.  Removing both 'walker' and
'declaringClass' brings a benefit to compressed oops as well (though not
for 16-byte aligned).

The size change, in bytes, for each execution mode is as follows:

                              32-bit: 32->24
                              64-bit: 56->40
                  64/compressed oops: 32->24
64/compressed oops, 16-byte aligned: 32->32

(For reference, the jol reports for the baseline and specimen are at [4]
and [5], respectively.)

Please review my code change for this.  The webrev is here:
http://cr.openjdk.java.net/~bchristi/8185925/webrev.03/

An automated test run is in progress.

Thanks!
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8153682
2. https://bugs.openjdk.java.net/browse/JDK-8185925
3. http://openjdk.java.net/projects/code-tools/jol/
4.
http://cr.openjdk.java.net/~bchristi/8185925/StackFrameInfo.jol.baseline.txt
5.
http://cr.openjdk.java.net/~bchristi/8185925/StackFrameInfo.jol.rmDeclClass.txt
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

Mandy Chung


On 11/6/17 4:23 PM, Brent Christian wrote:
> Please review my code change for this.  The webrev is here:
> http://cr.openjdk.java.net/~bchristi/8185925/webrev.03/
>

It's a good footprint improvement.  Thanks for doing this.

StackFrameInfo.java

   38     // Footprint improvement: MemberName::clazz can replace
   39     // StackFrameInfo::declaringClass.


The above comment can be removed.

41 private final boolean retainClassRef;

JVMS [1] has a note about Hotspot implementation of boolean array that
is encoded as a byte array. That explains JOL output that this boolean
field is 8-bit in our implementation.  This field could be changed to a
byte to hold additional flags, if any in the future.  It may be good to
change this to a byte making the field size explicit.

Otherwise looks good.

Mandy
[1]
https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-2.html#jvms-2.3.4
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

Mandy Chung


On 11/7/17 12:15 PM, Brent Christian wrote:
> Updated webrev here:
> http://cr.openjdk.java.net/~bchristi/8185925/webrev.04/

Nit: FLAGS should be lower-case (not a constant)

Otherwise looks good.   No need for a new webrev.

Mandy
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

coleen.phillimore
In reply to this post by Mandy Chung

Hi Brent,
The hotspot changes look good.
Coleen

On 11/7/17 3:15 PM, Brent Christian wrote:

> On 11/07/2017 09:45 AM, mandy chung wrote:
>>
>> StackFrameInfo.java
>>
>>   38     // Footprint improvement: MemberName::clazz can replace
>>   39     // StackFrameInfo::declaringClass.
>>
>> The above comment can be removed.
>
> Whoops - thanks.
>
>> 41 private final boolean retainClassRef;
>>
>> JVMS [1] has a note about Hotspot implementation of boolean array that
>> is encoded as a byte array.  That explains JOL output that this boolean
>> field is 8-bit in our implementation.  This field could be changed to a
>> byte to hold additional flags, if any in the future.  It may be good to
>> change this to a byte making the field size explicit.
>
> Thanks for the link and explanation.  I agree with making the change now.
>
> Updated webrev here:
> http://cr.openjdk.java.net/~bchristi/8185925/webrev.04/
>
> -Brent
>