RFR: 8031703 - Missing post-barrier in ReferenceProcessor

Per Liden per.liden at oracle.com
Tue Feb 4 05:14:54 PST 2014


Hi Thomas,

On 2014-02-04 13:45, Thomas Schatzl wrote:
> Hi Per,
>
> On Tue, 2014-02-04 at 11:15 +0100, Per Liden wrote:
>> Hi Tony,
>>
>> On 2014-02-04 04:20, Tony Printezis wrote:
>>> Hi Per,
>>>
>>> I went over the code one more time and I think your assessment is
>>> correct. But, could I recommend that you at least add a comment to
>>> that effect where the DiscoveredListIterator iter(...) is declared?
>> Hmm, which one? The iterator is instanciated in several places. I'd
>> suggest I add a comment like this to the remove() function, where the
>> pre-barriers is omitted. Ok?
>>
>>> Also, maybe, rename the discovered_list_needs_barrier parameter as
>>> ..._needs_post_barrier to make its role a bit more explicit?
>> Good idea, will add "post".
>>
>> Updated webrev here: http://cr.openjdk.java.net/~pliden/8031703/webrev.1/
>>
> As far as I can tell, the change looks good to me.

Thanks for looking at it!

>
> There are some minor comments about related code:
>
> In G1CollectedHeap::ref_processing_init(), there seems to be a
> copy&paste error when initializing the STW ref processor. I.e. the
> discovered_list_needs_post_barrier parameter is false, but the comment
> still reads:
>
>      // Setting next fields of discovered
>      // lists requires a barrier.
>
> which seems odd since we pass false to the parameter.

I agree that's a bit misleading. I have the feeling that whoever wrote 
that comment intended to describe the argument itself and not what it's 
set to in this particular case. I can adjust it while I'm at it. How about:

                                 // Setting next fields of discovered
                                 // lists does not require a barrier.

cheers,
/Per

>
> Thanks,
> Thomas
>
>



More information about the hotspot-gc-dev mailing list