<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix"><br>
      Hi John,<br>
      <br>
      This looks good to me.<br>
      <br>
      I think Vitaly has a point about the fact that we just "know" that
      <meta http-equiv="content-type" content="text/html;
        charset=ISO-8859-1">
      _parallel_workers == NULL is equivalent to
      parallel_marking_threads() == 0.<br>
      <br>
      I'm not going to insist on this, but would it make sense to add an
      assert to convey this information? I'm actually less worried about
      the case "parallel_marking_threads() > 0 but _parallel_workers
      == NULL" since that will result in a null de-reference that can be
      fairly easy to debug.<br>
      <br>
      But maybe this assert could be added at the start of the method:<br>
      <br>
        assert(_parallel_workers == NULL || parallel_marking_threads()
      > 0, "work gang not set up correctly"); <br>
      <br>
      I'm not sure we need that assert and I'm not convinced this is the
      right place to have it. But my thought is that this will detect an
      unexpected state that we would otherwise silently ignore.<br>
      <br>
      Bengt<br>
       <br>
      <br>
      This is probably not such a big problem if we <br>
      <br>
      On 1/10/13 1:47 AM, Vitaly Davidovich wrote:<br>
    </div>
    <blockquote
cite="mid:CAHjP37Hn=68a-xR439TKyC4LwSeaQ3WYSjE8uj+0adZ5+_KmOw@mail.gmail.com"
      type="cite">
      <p dir="ltr">Hi John,</p>
      <p dir="ltr">Thanks for the response.  Yeah, I figured it's the
        same thing since it's not null iff # of workers > 0. 
        However, if this relationship is ever broken or perhaps the gang
        can be set to null at some point even if workers > 0, then
        this code will segv again.  Hence I thought a null guard is a
        bit better, but it was just a side comment - code looks fine as
        is.</p>
      <p dir="ltr">Thanks</p>
      <p dir="ltr">Sent from my phone</p>
      <div class="gmail_quote">On Jan 9, 2013 7:41 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 Vitaly,<br>
            <br>
            Thanks for looking over the changes. AFAICT checking if
            _parallel_workers is not null is equivalent to checking that
            the number of parallel marking threads is > 0. I went
            with the latter check as other references to the parallel
            workers work gang are guarded by it. I'm not sure why the
            code was originally written that way but my guess is that,
            when originally written, the marking threads (like the
            concurrent refinement threads currently) were not in a work
            gang.<br>
            <br>
            Thanks,<br>
            <br>
            JohnC<br>
            <br>
            <div>On 1/8/2013 8:37 PM, Vitaly Davidovich wrote:<br>
            </div>
            <blockquote type="cite">
              <p dir="ltr">Hi John,</p>
              <p dir="ltr">What's the advantage of checking parallel
                marking thread count > 0 rather than checking if
                parallel workers is not NULL? Is it clearer that way?
                I'm thinking checking for NULL here (perhaps with a
                comment on when NULL can happen) may be a bit more
                robust in case it can be null for some other reason,
                even if parallel marking thread count is > 0.</p>
              <p dir="ltr">Looks good though.</p>
              <p dir="ltr">Thanks</p>
              <p dir="ltr">Sent from my phone</p>
              <div class="gmail_quote">On Jan 8, 2013 5:14 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
                  Everyone,<br>
                  <br>
                  Can I please have a couple of volunteers look over the
                  fix for this CR - the webrev can be found at: <a
                    moz-do-not-send="true"
                    href="http://cr.openjdk.java.net/%7Ejohnc/8005875/webrev.0/"
                    target="_blank">http://cr.openjdk.java.net/~johnc/8005875/webrev.0/</a><br>
                  <br>
                  Summary:<br>
                  One of the modules in the Kitchensink test generates a
                  VM_PrintThreads vm operation. The JVM crashes when it
                  tries to print out G1's concurrent marking worker
                  threads when ParallelGCThreads=0 because the work gang
                  has not been created. The fix is to add the same check
                  that's used elsewhere in G1's concurrent marking.<br>
                  <br>
                  Testing:<br>
                  Kitchensink with ParallelGCThreads=0<br>
                  <br>
                  Thanks,<br>
                  <br>
                  JohnC<br>
                </blockquote>
              </div>
            </blockquote>
            <br>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </body>
</html>