RFR: JDK-8086056: ParNew: auto-tune ParGCCardsPerStrideChunk

[Tony Printezis]

Thomas,

Latest version:

http://cr.openjdk.java.net/~tonyp/8086056/webrev.3/

See below for comments.

On July 2, 2015 at 4:43:42 AM, Thomas Schatzl (thomas.schatzl at oracle.com) wrote:

Hi Tony, 

On Wed, 2015-07-01 at 18:05 -0400, Tony Printezis wrote: 
> Latest changes, as discussed with Bengt and Thomas, 
> 
> http://cr.openjdk.java.net/~tonyp/8086056/webrev.2/ 

a few suggestions: 

- please use braces even for simple statements in if-clauses, e.g. 

parNewGeneration.cpp: 

883 if (!UseDynamicParGCStrides) return; 


I did that. But I actually added an extra verbose statement in the body of the if-statement (I thought it’d be helpful to also print the value of ParGCCardsPerStrideChunk when the dynamic calculation is off). So, I don’t have a simple return any more, but I’ll keep that in mind in the future (was it anywhere else? I didn’t immediately spot another occurrence of that).




- Comments should start with upper case: 

parNewGeneration.hpp: 

354 // automatically calculate ParGCCardsPerStrideChunk based on the 
old 


Done.




- parameter calculation in ParNewGeneration::adjust_cards_per_stride: 
- DynamicParGCStridesMinSize should be > 0 (only if ! 
UseDynamicParGCStrides is enabled, not sure if that can be encoded 
within the framework) 


If !UseDynamicParGCStrides, then both sets of min / max parameters are not used. I think it’d make the constraint checking a pain having to sanity-check parameters whether a flag is on or off (unless they mean different things in either case, of course).

I added a range check to the min / max stride size args, added a lower bound of 1, and decided to add an artificial upper bound of 32K (I have tested with up to 8K). Is that reasonable? We can revisit that if we need to increse it. Note that you can still set ParGCCardsPerStrideChunk to something higher if required.




- the issues with the Dynamic*Capacity variables Sangheon mentioned, 
although I think it is fine that they are equal, just that "r" needs to 
be set correctly. 


Yeah, fixed. (See previous e-mail.)




Just wanting to check back if it was intended that the code always 
rounds down, achieving the maximum stride only at the end of the 
interval. E.g. 

912 const int stride_size_log = log2_long((jlong) stride_size); 
913 res = (size_t) 1 << stride_size_log; 

It's fine with me, just asking. 


I did the simplest calculation. I can revise it if you want. Also note that if capacity > max_capacity, then stride size will be set to the max stride size.

Tony




Thanks, 
Thomas 









-----

Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com

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