<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Bengt,<br>
This seems to eliminate the TSO issues rather cleanly. One small
nit is that the comments in concurrentMarkThread.hpp and
CollectedHeap.cpp talk about setting/clearing 'flags' whereas now
it's setting a state variable.<br>
<br>
bill<br>
<br>
<div class="moz-cite-prefix">On 6/24/2015 3:28 AM, Bengt Rutisson
wrote:<br>
</div>
<blockquote cite="mid:558A5C2D.3020504@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<br>
<br>
Hi Vitaly,<br>
<br>
<div class="moz-cite-prefix">On 2015-06-23 23:53, Vitaly
Davidovich wrote:<br>
</div>
<blockquote
cite="mid:CAHjP37EEE=HZbgRvaTaCU=UueFqcKWOwEP0xBZBENHqZRmtyZQ@mail.gmail.com"
type="cite">
<p dir="ltr">Naive question - could this be converted into a
numeric state field that indicates the lifecycle (e.g 1 = in
progress, 2 = started)?</p>
</blockquote>
<br>
Good question! That's a much simpler and more stable solution.<br>
<br>
New webrev:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.02/">http://cr.openjdk.java.net/~brutisso/8129626/webrev.02/</a><br>
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
<br>
<blockquote
cite="mid:CAHjP37EEE=HZbgRvaTaCU=UueFqcKWOwEP0xBZBENHqZRmtyZQ@mail.gmail.com"
type="cite">
<p dir="ltr">sent from my phone</p>
<div class="gmail_quote">On Jun 23, 2015 5:42 PM, "bill pittore"
<<a moz-do-not-send="true"
href="mailto:bill.pittore@oracle.com">bill.pittore@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">
<div bgcolor="#FFFFFF" text="#000000"> Generally when you
have a storestore on the write side you need a loadload on
the read side to prevent the second read from floating
above the first one. The reading thread could read
in_progress as 0 before it reads starting. Meanwhile the
write thread writes 1 to in_progress, issues storestore,
clears starting. Reading thread then reads starting as 0.
I don't know if the CGC mutex somehow eliminates this
issue as I'm not familiar with the code in detail.<br>
<br>
bill<br>
<br>
<div>On 6/23/2015 4:25 PM, Bengt Rutisson wrote:<br>
</div>
<blockquote type="cite"> <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"
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/"
target="_blank">http://cr.openjdk.java.net/~brutisso/8129626/webrev.00/</a><br>
<br>
We need to add a barrier between the calls to
set_in_progress() and clear_started() to make sure that
other threads sees the correct value when they use
ConcurrentMarkThread::during_cycle().<br>
<br>
Thanks to Per and Bertrand for helping out identifying
and 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
ConcurrentMarkThread::_in_progress and they are meant to
overlap. That is, we should not set _started to false
until after we have set _in_progress to true. This is
done in sleepBeforeNextCycle(): <br>
<br>
void ConcurrentMarkThread::sleepBeforeNextCycle() { <br>
// We join here because we don't want to do the
"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
_in_progress (from set_in_progress()) is seen by other
threads after the write to _started (in
clear_started()). In that case there is a window when
during_cycle() may return false even though we are in a
concurrent cycle. <br>
<br>
<br>
Thanks,<br>
Bengt<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>