RFR: 8173764: Assert in G1 BOT is wrong
kim.barrett at oracle.com
Tue Feb 21 16:00:13 UTC 2017
> On Feb 21, 2017, at 6:11 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> On Tue, 2017-02-21 at 11:17 +0100, Stefan Johansson wrote:
>> Please review this fix for:
>> 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.
>> * 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
+1 on Thomas's comments. For the assert, make it say something like
"Object crossed region boundary", and then the kind of additional
information needed should become more obvious.
More information about the hotspot-gc-dev