<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Vitalty,<br>
<br>
<div class="moz-cite-prefix">On 1/23/2013 4:19 PM, Vitaly Davidovich
wrote:<br>
</div>
<blockquote
cite="mid:CAHjP37FkcsXuEq44XFSrgjWL0_oSUEf_8oZpV0JtWtyNN9-BVA@mail.gmail.com"
type="cite">
<p dir="ltr">Hi John,</p>
<p dir="ltr">Thanks for this explanation as well. I see what
you're saying about the concurrency control, but what I don't
understand is when this is called:</p>
<p dir="ltr">void reset_hot_cache() {<br>
107 _hot_cache_idx = 0; _n_hot = 0;<br>
108 }<br>
<br>
Since these are plain stores, what exactly ensures that they're
(promptly) visible to other GC threads? Is there some dependency
here, e.g. if you see _n_hot = 0 then _hot_cache_idx must also
be zero? I strongly suspect I missed the details in your
response that explain why this isn't a concern. Is there only a
particular type of thread that can call reset_hot_cache and/or
only at a certain point? It kind of sounds like it so don't know
if there's an assert that can be added to verify that.</p>
</blockquote>
<br>
At the point where this routine is called the GC workers have
finished the parallel phases of the GC and are idle. The thread that
is running is the VM thread. The rest of the VM is at a safepoint so
we are, in effect, single threaded. Yes there is an assert we can
add here:<br>
<br>
assert(SafepointSynchronize::is_at_safepoint() &&
Thread::current()->is_VM_thread(), "...");<br>
<br>
JohnC<br>
<br>
<blockquote
cite="mid:CAHjP37FkcsXuEq44XFSrgjWL0_oSUEf_8oZpV0JtWtyNN9-BVA@mail.gmail.com"
type="cite">
<p dir="ltr">Thanks</p>
<p dir="ltr">Sent from my phone</p>
<div class="gmail_quote">On Jan 23, 2013 5:51 PM, "John
Cuthbertson" <<a moz-do-not-send="true"
href="mailto:john.cuthbertson@oracle.com">john.cuthbertson@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">
Hi Vitaly,<br>
<br>
Thanks for looking over the code changes. I'll respond to your
other comments in a separate email. Detailed responses
inline....<br>
<br>
On 1/15/2013 4:57 PM, Vitaly Davidovich wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Hi John,<br>
<br>
Wow, that's a giant heap! :)<br>
<br>
I think G1ConcRSLogCacheSize needs to be validated to make
sure it's <= 31; otherwise, I think you get undefined
behavior on left shifting with it.<br>
<br>
</blockquote>
<br>
Good catch. Done.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
I don't think you need _def_use_cache -- can be replaced
with G1ConcRSLogCacheSize > 0?<br>
<br>
</blockquote>
<br>
Done. I've added a function that returns the result of the
comparison and I use that in place of G1ConcRSLogCacheSize.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
I'm sure this is due to my lack of G1 knowledge, but the
concurrency control inside g1HotCardCache is a bit unclear.
There's a CAS to claim the region of cards, there's a
HotCache lock for inserting a card. However,
reset_hot_cache() does a naked write of a few fields. Are
there any visibility and ordering constraints that need to
be enforced? Do some of the stores need an OrderAccess
barrier of some sort, depending on what's required? Sorry if
I'm just missing it ...<br>
<br>
</blockquote>
<br>
The drain routine is only called from within a GC pause but it
is called by multiple GC worker threads. Each worker will
claim a chunk of cards using the CAS and refine them.
Resetting the boundaries (the values reset by
reset_hot_cache()) in the drain routine would be a mistake
since a worker thread could see the new boundary values and
return, potentially leaving some cards unrefined and some
missing entries in remembered sets. I can only clear the
fields when the last thread has finished draining the cache.
The best place to do this is just before the VM thread
re-enables the cache (we know the worker threads will have
finished at this point). Since the "drain" doesn't actually
drain, perhaps a better name might be refine_all()?<br>
<br>
The HotCache lock is used when adding entries to the cache.
Entries are added by the refinement threads (and there will
most likely be more than one). Since the act of adding an
entry can also evict an entry we need the lock to guard
against hitting the ABA problem. This could result in skipping
the refinement of a card, which will lead to missing
remembered set entries which are not fun to track down.<br>
<br>
Draining during the GC is immune from the ABA problem because
we're not actually removing entries from the cache. We would
still be immune, however, if we were removing entries since we
would not be adding entries at the same time.<br>
<br>
Thanks,<br>
<br>
JohnC<br>
</blockquote>
</div>
</blockquote>
<br>
</body>
</html>