Request for review (s) - 7119584

[Igor Veresov]

Yes, this sounds right. Looks good. Thanks!

igor


On Friday, December 9, 2011 at 9:30 PM, Jon Masamitsu wrote:

> Igor,
> 
> I'm glad to describe this again.
> 
> The GC task manager creates the barrier task X. The GC worker
> is waiting to take tasks off a queue L. As soon as the task manager
> writes X to L, the worker can take X and start executing it. So I want
> the store of X to L to happen after the initializing stores to X
> have happened. That way the worker won't see X on L before
> it is safe to start executing X. Sound right?
> 
> Jon
> 
> On 12/9/2011 8:43 PM, Igor Veresov wrote:
> > Jon,
> > 
> > Since you are describing a dependency between stores and subsequent loads from other threads, shouldn't it be a "storeload" barrier?
> > 
> > igor
> > 
> > 
> > On Friday, December 9, 2011 at 1:19 PM, Jon Masamitsu wrote:
> > 
> > > 
> > > On 12/9/2011 1:01 PM, Vitaly Davidovich wrote:
> > > > Ok sounds like storestore is sufficient given your explanation - you don't
> > > > care whether the task is immediately visible to gc threads after it's
> > > > enqueued, you just want to make sure that once it is visible (might be
> > > > delayed due to store bufferring) that all fields are consistent.
> > > 
> > > 
> > > 
> > > Yes, that's correct.
> > > 
> > > > I'm obviously not on the hotspot dev team (just a curious bystander :)) but
> > > > perhaps you can change the code comment to indicate that you want ordered
> > > > writes but not necessarily immediately global visibility.
> > > 
> > > 
> > > 
> > > 
> > > Will do. Thanks again.
> > > 
> > > Jon
> > > > Thanks for the explanation,
> > > > 
> > > > Vitaly
> > > > On Dec 9, 2011 3:49 PM, "Jon Masamitsu"<jon.masamitsu at oracle.com (mailto:jon.masamitsu at oracle.com)> wrote:
> > > > 
> > > > > Vitaly,
> > > > > 
> > > > > This is what happens.
> > > > > 
> > > > > 1) create the barrier task
> > > > > 2) enqueue the barrier task to a list of tasks
> > > > > 3) make the tasks visible to the GC workers by added them to
> > > > > queue from which workers take tasks
> > > > > 
> > > > > At 3) a GC worker can take and execute the barrier task.
> > > > > I want all the fields in the barrier task to be in main memory
> > > > > and thus visible to the GC worker task when it starts to
> > > > > execute the barrier task. So by the time 3) happens
> > > > > (tasks are visible in the queue for the GC workers to
> > > > > take) the stores at 1) to be visible so that when the GC
> > > > > worker reads the barrier task, it is getting all the right
> > > > > values.
> > > > > 
> > > > > I think that's what the storestore does for me. Do I
> > > > > need something stronger? And thank you for thinking
> > > > > about this for me.
> > > > > 
> > > > > Jon
> > > > > 
> > > > > On 12/9/2011 9:48 AM, Vitaly Davidovich wrote:
> > > > > 
> > > > > > Hi Jon,
> > > > > > 
> > > > > > Are you sure your comment for the OrderAccess::storestore() matches this
> > > > > > barrier? Your comment mentions that you want global visibility but does
> > > > > > this need immediate visibility or you just want order for when it does
> > > > > > become visible? StoreStore is just the latter, but maybe that's what you
> > > > > > meant.
> > > > > > 
> > > > > > Regards,
> > > > > > 
> > > > > > Vitaly
> > > > > > On Dec 9, 2011 12:15 PM, "Jon Masamitsu"<jon.masamitsu@**oracle.com<jon.masamitsu at oracle.com (mailto:jon.masamitsu at oracle.com)>>
> > > > > > wrote:
> > > > > > 
> > > > > > 7119584: UseParallelGC barrier task can be overwritten.
> > > > > > > Just a few changed lines. The comments around the changes
> > > > > > > give an explanation in context so are a better explanation than
> > > > > > > I can summarize here.
> > > > > > > 
> > > > > > > http://cr.openjdk.java.net/~****jmasa/7119584/webrev.00/<http://cr.openjdk.java.net/~**jmasa/7119584/webrev.00/>
> > > > > > > <http:**//cr.openjdk.java.net/~jmasa/**7119584/webrev.00/<http://cr.openjdk.java.net/~jmasa/7119584/webrev.00/>
> > > > > > > Thanks.
> > > > > > 
> > > > > 
> > > > 
> > > 
> > 
>