RFR: 8069367: assert(_nextMarkBitMap->isMarked((HeapWord*) obj)) failed
kim.barrett at oracle.com
Tue Apr 14 02:28:40 UTC 2015
Thanks for looking at this.
On Apr 13, 2015, at 11:46 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> - for whatever reason the unified diff is broken, only containing the
> TEST.groups and the new test changes.
Maybe this bug?
> - I would have preferred the refactoring to be separate from the actual
> - why is the test excluded from the hotspot_gc target?
> - please avoid reformatting indentation of parameters to something
> non-standard (e.g. in G1CollectedHeap::initialize()).
For these, see earlier reply to Bengt.
> - I think the third paragraph is somewhat incomplete: the issue is not
> only the remaining remembered set entries, but that objects referenced
> from that object would need to be kept alive as well.
The first paragraph is about maintaining SATB invariants. The third
paragraph is about JDK-8071913. See reply to Bengt.
> - line 3474: I would prefer if the if does not start with the
> negation. While this case is arguably the more common one, there does
> not seem to be any performance concern at this point. Reading
> negations is always harder than the regular form.
I understand and agree with the preference for a non-negated test.
However, I wrote it that way because I liked all of the alternatives I
could think of less.
I don't like pre-clearing the candidates set for the same reason the
old code avoided clearing its set when there were no candidates; why
waste time doing unnecessary work. (A debug-only clear is unappealing
because that would be a side effect that is visible to non-debug code,
with the potential for debug and non-debug unintentionally having
I don't like just re-ordering the clauses because it puts the else
clause a long way from the test.
A change I considered but didn't take was to re-order the clauses but
extract most of the present else body into a helper function.
(Specifically, I'm thinking the remset purge to the dirty card queue
would be a good cut point.) I didn't do that because that kind of
code motion makes review harder. I'd be fine with doing that as a
followup, or even as part of this change if that's preferable to
> Calculation of the hrm_index() could be factored out for both cases.
It could, but I don't think that's helpful in the present form. If
the remset purge were moved to a helper function then it would make
more sense, with all uses being textually close together.
More information about the hotspot-gc-dev