RFR(s): 8152176: Big ParGCCardsPerStrideChunk values can cause overflow for CMS GC (original) (raw)

sangheon sangheon.kim at oracle.com
Tue Mar 29 22:42:18 UTC 2016

Thanks, Jon.

Sangheon

On 03/29/2016 02:13 PM, Jon Masamitsu wrote:

On 3/29/2016 12:07 PM, sangheon wrote: Hi Jon,

Thank you for reviewing this. On 03/29/2016 10:49 AM, Jon Masamitsu wrote: Sangheon,

I agree that we don't want to expose CardTableModRefBS::lastvalidindex but I'd consider making CardTableModRefBS::cardsrequired() public. That would remove the need to duplicate the calculation and it seems a reasonable question to ask the card table implementation how many cards it is going to require. I'll let other disagree if they like. Good idea and changed to 'public'.

Add in the message a little more about why the value is too large.

396 CommandLineError::print(verbose, 397 "ParGCCardsPerStrideChunk (" INTXFORMAT ") "is too large for the heap size and " "must be " 398 "less than or equal to card table size (" SIZEFORMAT ")\n", 399 value, cardtablesize); Fixed. I think a little more information here would help future maintainers so instead of 403 // If nstrides which is used with ParGCCardsPerStrideChunk is really large, we would face an overflow. I updated the comment. This statement can overflow? 404 uintx nstrides = ParallelGCThreads * ParGCStridesPerThread; No. Because above statement is checked not to overflow from ParGCStridesPerThreadConstraintFunc() which is called before this constraint function. (added this explanation as well) Ok. Updated webrev: http://cr.openjdk.java.net/~sangheki/8152176/webrev.01/ http://cr.openjdk.java.net/~sangheki/8152176/webrev.01to00/ Looks good. Jon Thanks, Sangheon

Jon On 3/28/2016 3:50 PM, sangheon wrote: Hi all, Could I have some reviews for this change? As very large value of ParGCCardsPerStrideChunk flag would result in an overflow, we need a constraint function after memory initialization. And the function will check the flag to be less than of equal to the size of card table and not to make an overflow with other stride factors(ParallelGCThreads and ParGCStridesPerThread). CR: https://bugs.openjdk.java.net/browse/JDK-8152176 Webrev: http://cr.openjdk.java.net/~sangheki/8152176/webrev.00 Testing: JPRT, all runtime/commandline JTREG tests on all platforms Thanks, Sangheon

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

More information about the hotspot-gc-dev mailing list