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

[Jesper Wilhelmsson]

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?
/Jesper


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.org/pipermail/hotspot-gc-dev/attachments/20120227/8cc44c8e/jesper_wilhelmsson.vcf>