RFR: 8155257: ParNew/CMS: Clean up promoted object tracking

[Jon Masamitsu]

Tony,

Changes look good.  Couple of small points.

http://cr.openjdk.java.net/~tonyp/8155257/webrev.0/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp.frames.html

1097 // This method should be called at the end of the main ParNew
1098 // parallel phase to collapse the promoted object lists. Given that
1099 // we don't want promoted objects to be tracked in future phases
1100 // (e.g., during reference processing) we also disable promote
1101 // object tracking here.

The placement of this block comment was slightly confusing.  I wasn't
sure if "This method ..." applied specirfically  to the 
stopTrackingPromotions()
until the last part "also disable ...".   Would it be better placed 
before the method.

2132   // Also reset promotion tracking in par gc thread states.
2133 // I don't think this is really needed, as promotion tracking should
2134 // have already been disabled. However, it sanity checks that the
2135 // promotion lists are empty so I think it's helpful to leave it in.

The comment makes clear you intent but  is there a simpler and more direct
sanity check you can use?  Comments sometimes get lost in the fog of code
churn and calling a method just for sanity checkinf seems wasteful and 
confusing
(Why is stopTrackingPromotions() call twice?  Is there something that the
first all didn't do? Yes, you comment explains it but I have to read the 
comment.)
If you agree you can fix it at a later time.  If you disagree, I'll 
silently forget about it :-).

Jon

On 04/27/2016 01:40 PM, Tony Printezis wrote:
> Small changes to clean up when promoted object tracking is enabled / 
> disabled and when tearing down the promoted object lists is done:
>
> http://cr.openjdk.java.net/~tonyp/8155257/webrev.0/ 
> <http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.0/>
>
> I also had to amend an assert which was too strong and removed the 
> worker_id argument from the stopTrackingPromotions() method as it was 
> actually not used.
>
> I’ll run more testing overnight.
>
> Tony
>
> -----
>
> Tony Printezis | JVM/GC Engineer / VM Team | Twitter
>
> @TonyPrintezis
> tprintezis at twitter.com <mailto:tprintezis at twitter.com>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160428/b98aef6d/attachment.htm>