RFR: G1HRRSFlushLogBuffersOnVerify with remembered set verification does not work
thomas.schatzl at oracle.com
Thu Jun 29 09:37:36 UTC 2017
On Thu, 2017-06-29 at 11:11 +0200, Erik Helin wrote:
> Hi all,
> this patch removes the developer flag
> This flag has been broken for some time and I don't see any reason
> for keeping it. The flag is `false` by default so I guess this code
> isn't exercised all that much :/
> I assume that the original intent of the flag was to perform an
> "update rs" phase before doing remembered set (rem set) verification.
> Due to the "update rs" phase, all rem sets would be complete, so
> verification would verify more rem set entries. However, since this
> code was added, update_rs has changed quite a bit, and this code
> hasn't kept up. It is no longer possible to call update_rs in the way
> this code expects.
> Instead of spending time on trying to get this code up to date, I
> suggest we just remove it. During verification after a collection
> (young or mixed) we already do this kind of rem set verification
> (since all rem sets must then be complete since all collections
> currently do update_rs). If we are worried that we verify too few rem
> set entries during e.g. remark and cleanup, then we could for example
> run with very aggressive concurrent refinement.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8153360
> Patch: http://cr.openjdk.java.net/~ehelin/8153360/00/
> Test: make hotspot - this is "just" removal of code
Please add a comment about what the last clause in the verification
code actually means (heapRegion.cpp:584). Something like:
// Reference may not have been refined into the remembered sets yet.
// Instead of looking into all dirty card queues, we take a shortcut
// by looking at whether the corresponding card is dirty.
// ObjArrays may either by marked on the object header or exactly.
(Actually I would guess the "correct" clause here would be is_array()
and not is_objArray(), but primitive type arrays are never marked as
they do not contain references)
I do not need a re-review for the comment change.
More information about the hotspot-gc-dev