RFR (M): 8200545: Improve filter for enqueued deferred cards

Thomas Schatzl thomas.schatzl at oracle.com
Thu Apr 25 07:33:21 UTC 2019


Hi all,

  ErikD had some improvement suggestions during an internal review:

- he suggested to introduce a getter that returns you a string for the
region type. This allows removal of the *FORMAT defines and generally
looks better.

- also added typedefs for both members of the G1HeapRegionAttr for
uniformity.

- removed some confusing comments, and some debug code.

The changes are based on the "full" version further below. So it is
probably still useful to look at the "webrev" changes first to review
the important differences.

http://cr.openjdk.java.net/~tschatzl/8200545/webrev.0_to_1/
http://cr.openjdk.java.net/~tschatzl/8200545/webrev.1/

Thanks,
  Thomas

On Mon, 2019-04-15 at 09:29 +0200, Thomas Schatzl wrote:
> Hi all,
> 
>   can I have reviews for this change that improves the check whether
> the destination region for a reference has a remembered set that
> needs updating?
> 
> This removes the need to use two distinct closures for scanning oops
> during evacuation, as both the new and the old variant are now equal
> in performance, so not only simplifying existing code, but also
> simplifying the follow-up change JDK-8213108 a lot.
> 
> The idea of this change is to put the information whether a region
> needs remembered set updates into the InCSetState table. This results
> in that for every reference we do not need to (re-)calculate the
> destination region, and then do the walk across several data
> structures (HeapRegion->HeapRegionRemSet) every time - this simply
> combines this with the existing calculation of InCSetState.
> 
> The code adds a new member to InCSetState that indicates whether the
> region needs remembered set updates.
> 
> As a second, seperate step, there is a change that renames
> InCSetState in a imho more understandable G1RegionAttr(ibutes); if
> wanted I can do this in a separate CR, but here I will just present
> them as separate webrevs.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8200545
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8200545/webrev/ (add InCSetState
> member only)
> http://cr.openjdk.java.net/~tschatzl/8200545/webrev.rename/ (rename
> InCSetstate only)
> http://cr.openjdk.java.net/~tschatzl/8200545/webrev.full/ (full
> change)
> Testing:
> hs-tier1-5, internal perf testing;
> 
> While testing I found JDK-8222426 - however that one also occurs
> without the changes, so I think we can go ahead with this regardless.
> 
> Thanks,
>   Thomas
> 
> 
> 
> 





More information about the hotspot-gc-dev mailing list