<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 24/06/15 13:39, Vitaly Davidovich
wrote:<br>
</div>
<blockquote
cite="mid:CAHjP37E1Q-1NVu4Gq3C+F5T=pB4X65+CtQP+eM8Np3YgZwL4MQ@mail.gmail.com"
type="cite">
<p dir="ltr">Looks good Bengt (not a Reviewer though).</p>
</blockquote>
<br>
Thanks, Vitaly!<br>
<br>
Bengt<br>
<br>
<blockquote
cite="mid:CAHjP37E1Q-1NVu4Gq3C+F5T=pB4X65+CtQP+eM8Np3YgZwL4MQ@mail.gmail.com"
type="cite">
<p dir="ltr">sent from my phone</p>
<div class="gmail_quote">On Jun 24, 2015 6:29 AM, "Bengt Rutisson"
<<a moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>>
wrote:<br type="attribution">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
On 2015-06-24 11:27, Bertrand Delsart wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
Looks good... and much simpler :-)<br>
</blockquote>
<br>
Thanks, Bertrand!<br>
<br>
Bengt<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Bertrand.<br>
<br>
On 24/06/2015 10:31, Bengt Rutisson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
On 2015-06-24 10:34, Per Liden wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
On 2015-06-24 10:15, Bengt Rutisson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Hi Per,<br>
<br>
Thanks for looking at this!<br>
<br>
On 2015-06-24 10:09, Per Liden wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Bengt,<br>
<br>
On 2015-06-24 09:28, Bengt Rutisson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Hi Vitaly,<br>
<br>
On 2015-06-23 23:53, Vitaly Davidovich wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0
0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<br>
Naive question - could this be converted into a
numeric state field<br>
that indicates the lifecycle (e.g 1 = in
progress, 2 = started)?<br>
<br>
</blockquote>
<br>
Good question! That's a much simpler and more
stable solution.<br>
<br>
New webrev:<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.02/"
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~brutisso/8129626/webrev.02/</a><br>
</blockquote>
<br>
This looks much nicer, and I can't think of any
reason why that<br>
wouldn't work. One little request though, can we
name the states to<br>
match the function names, like
Idle/Started/InProgress? And<br>
clear_in_progress() should probably be set_idle() to
align with the<br>
rest.<br>
</blockquote>
<br>
Yes, I was struggling a bit with the naming. What do
you think about<br>
this?<br>
<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.03/"
rel="noreferrer" target="_blank">cr.openjdk.java.net/~brutisso/8129626/webrev.03/</a><br>
</blockquote>
<br>
Looks good!<br>
</blockquote>
<br>
Thanks Per!<br>
<br>
Bengt<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
cheers,<br>
/Per<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
Bengt<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
cheers,<br>
/Per<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0
0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
sent from my phone<br>
<br>
On Jun 23, 2015 5:42 PM, "bill pittore" <<a
moz-do-not-send="true"
href="mailto:bill.pittore@oracle.com"
target="_blank">bill.pittore@oracle.com</a><br>
<mailto:<a moz-do-not-send="true"
href="mailto:bill.pittore@oracle.com"
target="_blank">bill.pittore@oracle.com</a>>>
wrote:<br>
<br>
Generally when you have a storestore on the
write side you need a<br>
loadload on the read side to prevent the
second read from<br>
floating<br>
above the first one. The reading thread
could read in_progress as<br>
0 before it reads starting. Meanwhile the
write thread writes<br>
1 to<br>
in_progress, issues storestore, clears
starting. Reading thread<br>
then reads starting as 0. I don't know if
the CGC mutex somehow<br>
eliminates this issue as I'm not familiar
with the code in<br>
detail.<br>
<br>
bill<br>
<br>
On 6/23/2015 4:25 PM, Bengt Rutisson wrote:<br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<br>
Hi everyone,<br>
<br>
Could I have a couple of reviews for this
change?<br>
<br>
<a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8129626"
rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8129626</a><br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.00/"
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~brutisso/8129626/webrev.00/</a><br>
<<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.00/"
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.00/</a>><br>
<br>
We need to add a barrier between the calls
to set_in_progress()<br>
and clear_started() to make sure that
other threads sees the<br>
correct value when they use<br>
ConcurrentMarkThread::during_cycle().<br>
<br>
Thanks to Per and Bertrand for helping out
identifying and<br>
sorting this out.<br>
<br>
From the bug report:<br>
<br>
ConcurrentMarkThread::during_cycle() is
implemented as:<br>
<br>
bool during_cycle() { return started() ||
in_progress(); }<br>
<br>
So, it checks both
ConcurrentMarkThread::_started and<br>
ConcurrentMarkThread::_in_progress and
they are meant to<br>
overlap.<br>
That is, we should not set _started to
false until after we have<br>
set _in_progress to true. This is done in<br>
sleepBeforeNextCycle():<br>
<br>
void
ConcurrentMarkThread::sleepBeforeNextCycle() {<br>
// We join here because we don't want to
do the<br>
"shouldConcurrentMark()"<br>
// below while the world is otherwise
stopped.<br>
assert(!in_progress(), "should have been
cleared");<br>
<br>
MutexLockerEx x(CGC_lock,
Mutex::_no_safepoint_check_flag);<br>
while (!started() &&
!_should_terminate) {<br>
CGC_lock->wait(Mutex::_no_safepoint_check_flag);<br>
}<br>
<br>
if (started()) {<br>
set_in_progress();<br>
clear_started();<br>
}<br>
}<br>
<br>
On non-TSO platforms there is a risk that
the write to<br>
_in_progress (from set_in_progress()) is
seen by other threads<br>
after the write to _started (in
clear_started()). In that case<br>
there is a window when during_cycle() may
return false even<br>
though we are in a concurrent cycle.<br>
<br>
<br>
Thanks,<br>
Bengt<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
</blockquote>
</div>
</blockquote>
<br>
</body>
</html>