(resend) Request for review (S): 7068625 Testing 8 bytes of card table entries at a time speeds up card-scanning

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Mon Feb 27 13:32:20 UTC 2012

Looks good!

A few minor details:

In cardTableModRefBS.hpp, maybe it's just me (after all, it's Monday) but I 
don't understand the comment:

"a word worth row of clean card values"

 From the code I understand what the constant is used for, but it could be more 
explicitly said in the comment. Maybe something like:

"A full word with clean card marks. Used for fast traversal of empty regions in 
the card table."

In cardTableRS.hpp, again in the comment:

"making this constants"

Should it be "this constant" or "these constants"?
On which platform did this kill performance? I think this should be explicitly 
stated. How much has this been verified?

On 2012-02-24 13:53, Bengt Rutisson wrote:
> On 2012-02-24 13:50, Jesper Wilhelmsson wrote:
>> I can have a look at it but I don't have the time to do it today. If Monday
>> is OK then I can take it.
> Thanks, Jesper!
> Will also need a second review from someone with OpenJDK reviewer status.
> Bengt
>> /Jesper
>> 24 feb 2012 kl. 13:14 skrev Bengt Rutisson <bengt.rutisson at oracle.com
>> <mailto:bengt.rutisson at oracle.com>>:
>>> Hi all,
>>> Just pinging this review request. Does anybody have some time to look at it?
>>> It is a fairly small and straight forward change...
>>> Thanks,
>>> Bengt
>>> -------- Original Message --------
>>> Subject: 	Request for review (S): 7068625 Testing 8 bytes of card table
>>> entries at a time speeds up card-scanning
>>> Date: 	Tue, 21 Feb 2012 12:03:50 +0400
>>> From: 	Alexey Ragozin <alexey.ragozin at gmail.com>
>>> To: 	hotspot-gc-dev at openjdk.java.net
>>> CC: 	Bengt Rutisson <bengt.rutisson at oracle.com>
>>> Hi,
>>> I would like few volunteers to review changes for
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7068625
>>> WebRev: http://cr.openjdk.java.net/~brutisso/7068625/webrev.00/
>>> <http://cr.openjdk.java.net/%7Ebrutisso/7068625/webrev.00/>
>>> Change summary
>>> For large heaps (I was focusing on 8GiB and above) it is common to have long
>>> continuous ranges of clean cards.
>>> Patch is introducing a short path for skipping ranges of clean cards using
>>> word aligned memory access instead of byte aligned.
>>> Patch affects serial and CMS collectors. For CMS collector stride size
>>> should be increase to see any performance gains (I was using
>>> -XX:+UnlockDiagnosticVMOptions
>>> -XX:ParGCCardsPerStrideChunk=4096)
>>> For testing I was mainly using synthetic benchmark randomly modifying hash
>>> tables in heap, thus uniformly touching cards across heaps.
>>> Average duration of young GC pause were used as KPI.
>>> More details about testing can be found at
>>> http://blog.ragozin.info/2011/07/openjdk-patch-cutting-down-gc-pause.html
>>> Though article is referring jdk6, my resent tests with trunk jdk7 show no
>>> difference.
>>> I was also tested patch with real application (Oracle Coherence storage node).
>>> With 16GiB of heap and CMS/ParNew GC, enabling patch have shortened GC
>>> pauses roughly in 2 times.
>>> Source code of benchmark used in test are available at
>>> https://gridkit.googlecode.com/svn/branches/aragozin-sandbox/young-gc-bench
>>> Main class YoungGCPauseBenchmark
>>> Regards,
>>> Alexey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jesper_wilhelmsson.vcf
Type: text/x-vcard
Size: 263 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20120227/8cc44c8e/jesper_wilhelmsson.vcf>

More information about the hotspot-gc-dev mailing list