RFR (S): JDK-6991197 G1: specialize deal_with_reference() for narrowOop*

Tony Printezis tprintezis at twitter.com
Thu Jan 30 16:21:01 UTC 2014


Thomas,

Thumbs up on the change, as is. As I said, the other two points were 
optional.

Tony

On 1/30/14, 6:46 AM, Thomas Schatzl wrote:
> Hi Tony,
>
>    thanks for the review :)
>
> On Wed, 2014-01-29 at 15:34 -0500, Tony Printezis wrote:
>> Thomas,
>>
>> (if you need one more opinion) The latest webrev also looks good to me
>> and I found it a nice improvement over the first one.
>>
>> Can I take it a step (OK: two steps!) further, if you're interested (and
>> feel free to ignore this)?
>>
>> Why is clear_partial_array_mask() a template given that it should only
>> work on oops*'s? (correct?)
> Cannot do that right away, since the compiler always instantiates
> G1ParScanPartialArrayClosure::do_oop_nv() with both narrowOop and oop*.
> It does not detect at parse time that has_partial_array_mask() always
> returns false for narrowOop*.
>
>> Then, why is G1ParScanPartialArrayClosure::do_oop_nv(T* p) a template
>> too (should only be called for T == oop). In fact, why is it a closure
>> at all? Would it make sense to just move its body to a method with an
>> oop* parameter and just call it directly from deal_with_reference()?
> I am currently working on JDK-8027547 to change the array chunk
> iteration code to something like you suggest. Is it okay for you to
> defer these cleanup changes for that CR (or another CR)?
>
> Thanks for the information,
>    Thomas
>
>

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

@TonyPrintezis
tprintezis at twitter.com




More information about the hotspot-gc-dev mailing list