RFR: 8209163: SA: Show Object Histogram asserts with ZGC

Per Liden per.liden at oracle.com
Thu Sep 13 10:48:03 UTC 2018


Hi JC,

Thanks for reviewing.

On 09/12/2018 06:02 PM, JC Beyler wrote:
> Hi Per,
> 
> I do not know this code but your need to 
> call heapIterationFractionUpdate seems to be a code smell that something 
> else could be fixed, no?
> 
> Your webrev looks fine to me if I ignore the code smell that comes from 
> having to call the update call:
> 
> When I look at 
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/HSDB.java, the 
> reason it requires that first call is, like you said, that the frame is 
> created only at the first heapIterationFractionUpdate. However, could we 
> not test frame and if it is not created, don't try to remove it?

I actually did that initially, but had the feeling people would think 
that was an even worse code smell ;) I don't have a strong opinion on 
this, so I updated the webrev to check for null instead.

http://cr.openjdk.java.net/~pliden/8209163/webrev.1

cheers,
Per

> 
> Thanks,
> Jc
> 
> On Wed, Sep 12, 2018 at 1:25 AM Per Liden <per.liden at oracle.com 
> <mailto:per.liden at oracle.com>> wrote:
> 
>     This patch avoids an assertion, and instead prints a warning, when
>     trying to show the "Object Histogram" when using ZGC. I also had to add
>     a call to heapIterationFractionUpdate() so that the update progress
>     frame is properly created, even when there are not heap regions to walk
>     over. Without this, you only get a call to heapIterationCompleted(),
>     with a null frame, which throws a NullPointerException. Always making a
>     call to heapIterationFractionUpdate(0.0) in the prologue avoids this by
>     making sure we always created the frame, even if no more fraction
>     updates will happen.
> 
>     Bug: https://bugs.openjdk.java.net/browse/JDK-8209163
>     Webrev: http://cr.openjdk.java.net/~pliden/8209163/webrev.0
> 
>     /Per
> 
> 
> 
> -- 
> 
> Thanks,
> Jc


More information about the serviceability-dev mailing list