RFR: 8173764: Assert in G1 BOT is wrong

Thomas Schatzl thomas.schatzl at oracle.com
Tue Feb 21 11:11:10 UTC 2017


Hi,

On Tue, 2017-02-21 at 11:17 +0100, Stefan Johansson wrote:
> Hi,
> 
> Please review this fix for:
> https://bugs.openjdk.java.net/browse/JDK-8173764
> 
> Webrev:
> http://cr.openjdk.java.net/~sjohanss/8173764/hotspot.00/
> 
> Summary:
> The changed assert, asserted that there was an object at the start of
> the heap. What we want to assert is that no object span over region 
> boundaries. That basically means that there is an object at the start
> of every region, with one exception, continues humongous regions.
> They need special handling since they don't contain an object at the
> start of the region.
> 
> Testing:
> * JPRT
> * RBT tier2 + tier3

Some comments:
 - maybe the CR title could be a bit more specific, like "G1 BOT
wrongly assumes that objects must always begin at the start of
G1BlockOffsetTablePart". Sure, that is quite long...

 - could the change make the _object_can_span variable assert-only? I
prefer if that variable were only set when it's used. It also makes it
clear that it is some variable used for assertions only.

 - the variable should have a comment what it is supposed to indicate,
not only a description of one of it's values. Maybe something like
"Indicates that an object can span/reach into
this G1BlockOffsetTablePart".

 - the setter should have the same name as the variable to set. The
current name kind of introduces a higher level concept ("humongous")
into this code unnecessarily.

 - Maybe the assert could print the value it found instead of a zero.

 - copyright updates

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list