RFR: 8166811: Missing memory fences between memory allocation and refinement
Kim Barrett
kim.barrett at oracle.com
Wed Nov 16 18:02:07 UTC 2016
> On Nov 16, 2016, at 4:06 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>
> Hi Kim,
>
> On Tue, 2016-11-15 at 19:00 -0500, Kim Barrett wrote:
>> I've updated some comments to mention that external synchronization.
>
> 581 // The region could be young. Cards for young regions are set
> to
> 582 // g1_young_gen, so the post-barrier will filter them
> out. However,
> 583 // that marking is performed concurrently. A write to a young
> 584 // object could occur before the card has been marked young,
> slipping
> 585 // past the filter.
>
> I would prefer if the text would not change terminology for the same
> thing mid-paragraph, from "setting" to "marking". The advantage of it
> reading better seems to be smaller than the potential confusion.
// The region could be young. Cards for young regions are
// distinctly marked (set to g1_young_gen), so the post-barrier will
// filter them out. However, that marking is performed
// concurrently. A write to a young object could occur before the
// card has been marked young, slipping past the filter.
Better?
>
> Everything else looks very nice.
>
> Thanks for considering my comments.
Thanks, and thank you for reviewing so carefully.
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8166811
>>
>> Webrevs:
>> full: http://cr.openjdk.java.net/~kbarrett/8166811/webrev.01/
>> incr: http://cr.openjdk.java.net/~kbarrett/8166811/webrev.01.inc/
>>
>> Also, since this set of changes is rather intertwined with the
>> changes
>> for 8166607, here is a combined webrev for both:
>> http://cr.openjdk.java.net/~kbarrett/8166811/combined.01/
>>
>> I think I'll do as Erik suggested and push the two together.
>
> Just fyi, you can push two commits at once, or one commit having two
> CR-number lines.
> I think it is sufficient to commit these two changes in a single push
> job, but I do not see a need for making it a single commit.
>
> Either way is fine with me.
Perhaps I sowed confusion with the combined webrev. The purpose
of that was to make it easy to see the combined effect of the two
changes. I’m planning to do one push of two change sets.
>
> Thanks,
> Thomas
More information about the hotspot-dev
mailing list