<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi Vitaly.<br>
    <br>
    I'm not sure it's an issue. I don't recall seeing any unflushed
    writes from during the GC when the VM leaves the safepoint even on
    an RMO architecture (Itanium).<br>
    <br>
    JohnC<br>
    <br>
    <div class="moz-cite-prefix">On 1/23/2013 4:41 PM, Vitaly Davidovich
      wrote:<br>
    </div>
    <blockquote
cite="mid:CAHjP37HB1pQVCTr0QHXBQ2x10evHJRgMauEz5UqK5Sdnih3TwQ@mail.gmail.com"
      type="cite">
      <p dir="ltr">Got it now - thanks.  So then does exiting the
        safepoint guarantee that these writes are flushed to memory so
        next time GC threads run they see 0s? Or is that not
        important/enforced elsewhere?</p>
      <p dir="ltr">Sent from my phone</p>
      <div class="gmail_quote">On Jan 23, 2013 7:36 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">
          <div text="#000000" bgcolor="#FFFFFF"> Hi Vitalty,<br>
            <br>
            <div>On 1/23/2013 4:19 PM, Vitaly Davidovich wrote:<br>
            </div>
            <blockquote 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 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"
                  target="_blank">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>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </body>
</html>