<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix"><font size="-1">Hi Brad,<br>
        <br>
        Thanks for your good suggestions. I have fixed most of them and
        re-uploaded my changes at
        <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dxu/7177045.01/">http://cr.openjdk.java.net/~dxu/7177045.01/</a>.<br>
        <br>
        The reason that I chose </font>
      <meta http-equiv="content-type" content="text/html;
        charset=ISO-8859-1">
      <font size="-1">ArrayDeque instead of LinkedList is that </font><font
        size="-1">ArrayDeque</font><font size="-1"> seems have better
        performance. According to the java doc, "most ArrayDeque
        operations run in amortized constant time" and "this class is
        likely to be faster then LinkedList when used as a queue." It is
        also very easy to remove last elements to back off memory
        allocation.<br>
        <br>
        In addition, I did not switch to diamond operator. Because old
        Jdk bundles, say jdk 1.7.0-ea-b23 and jdk 1.7.0-ea-b29 used in
        my testing, failed to compile diamond operator. Here are the
        compilation error messages,<br>
      </font>
      <blockquote><font size="-1">TestProviderLeak.java:62: illegal
          start of type</font><br>
        <font size="-1">        Deque<byte []> data = new
          ArrayDeque<>();</font><br>
        <font size="-1">                                             ^</font><br>
        <font size="-1">1 error</font><br>
      </blockquote>
      <font size="-1"><br>
        I guess those jdk might be too early to adopt the diamond
        operator changes. I am not sure whether we still take these old
        jdk bundles into consideration here. Thanks!<br>
        <br>
        -Dan<br>
        <br>
      </font>On 06/28/2012 05:30 PM, Brad Wetmore wrote:<br>
    </div>
    <blockquote cite="mid:4FECF72A.6090102@oracle.com" type="cite">Dan,
      congrats on assembling and posting your first webrev.  Besides the
      big picture things, since you are new, I'll also be looking for
      minor things that you may or may not know yet. <br>
      <br>
      On 6/28/2012 1:49 PM, Dan Xu wrote: <br>
      <blockquote type="cite">Security code reviewers, <br>
        <br>
        I have fixed a security test failure and posted my changes at <br>
        <a class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Edxu/7177045/">http://cr.openjdk.java.net/~dxu/7177045/</a>.
        Please help review it. Thanks! <br>
      </blockquote>
      <br>
      Minor nit:  line 38 has a space at the end of the line.  Current
      jstyle guidelines state no indention with tabs and no whitespace
      at the end of the lines. <br>
      <br>
      Lines 61/89:  memroy->memory <br>
      <br>
      Just wondering why you chose a Deque instead of a simpler
      LinkedList? <br>
      <br>
      Suggest more liberal use of comments, either in the method's
      comments or inline.  Good to explain your assumptions/approach in
      case things aren't obvious.  For example, why do you backoff 3MB
      after allocating available memory?  And at line 134:  the
      operation could either time out or threw an exception.  Nice to
      make that clear. <br>
      <br>
      dummyData could be a local variable. <br>
      <br>
      Line 64/113:  consider using the JDK 7 diamond <> operator
      on your generics. <br>
    </blockquote>
    <blockquote cite="mid:4FECF72A.6090102@oracle.com" type="cite"> <br>
      Line 114:  consider adding a @overrides annotation on the call()
      method. <br>
      <br>
      Line 139:  I'm being paranoid here, but shutdownNow doesn't
      guarantee threads will be stopped.  If we actually got into a
      situation where there was a timeout, executor.shutdownNow() *may*
      never return.  One reason is it might be hanging somewhere waiting
      for memory.  I would suggest as part of your finally block, you
      dequeue all the memory in dummyData, call System.gc(), then run
      executor.shutdownNow(). JTREG will timeout after two minutes, but
      if we can proactively help the situation, we might as well. <br>
      <br>
      Otherwise, looks good.  We'll wait to see if anyone has other
      thoughts, and if not, we'll push when you're back from vacation. <br>
      <br>
      Brad <br>
    </blockquote>
    <br>
    <br>
  </body>
</html>