RFR: 8166663: Simplify oops_on_card_seq_iterate_careful interface

Thomas Schatzl thomas.schatzl at oracle.com
Mon Sep 26 08:06:58 UTC 2016


Hi Kim,

On Sun, 2016-09-25 at 19:57 -0400, Kim Barrett wrote:
> Please review this simplification of the interface and implementation
> of HeapRegion::oops_on_card_seq_iterate_careful. It only has one
> caller, G1RemSet::refine_card, which does not use some of the
> presently supported generality.
> 
> By making these changes we simplify the analysis and code changes
> required to address the main task of JDK-8160369.
>  
> Specific issues include:
> 
> (1) This function has a bool filter_young parameter, which is used to
> control and conditionalize its behavior. However, the only caller
> always provides a true value for that parameter.
> 
> (2) This function returns either NULL (to indicate success) or the
> start address of a possibly incompletely initialized object that
> caused heap parsing to fail. The caller uses this value to decide
> whether the operation was successful (NULL), but doesn't use the
> actual failure value (the address where parsing failed). Instead,
> there is a comment suggesting a more complex to implement response to
> failure might be used in the future.
> 
> Unlike CMS, with G1 the situation where parsing may fail is quite
> rare. This will be discussed further with the fix for JDK-8166607. So
> extra work here isn't worthwhile, and the present response of just
> requeuing the card for later reconsideration is adequate.
> 
> Since the caller presently only uses the result as a (inverted)
> boolean success/failure, changing the result to a (non-inverted)
> success/failure indication further simplifies both the caller and the
> callee.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8166663
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8166663/webrev.00/
> 
> Testing:
> JPRT, local jtreg hotspot_all
> 

  - please add braces around the statement in the statement executed
when the boolean expression is true in line 369 heapRegion.cpp.

  - maybe add a sentence like "Iterate over the the card in the card
table specified by card_ptr/mr, applying cl on all references." at the
description in line 656 in heapRegion.hpp. Or just try to make the
description a bit more readable.

In any case I do not need re-review for either trivial change.

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list