<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hi Aleksey,<br>
    <br>
    Adding hotspot-runtime-dev, since this is adding tests to
    test/runtime.<br>
    <br>
    Inlined:<br>
    <br>
    On 2013-10-24 16:00, <a class="moz-txt-link-abbreviated" href="mailto:aleksey.timofeev@oracle.com">aleksey.timofeev@oracle.com</a> wrote:<br>
    <blockquote cite="mid:526927FB.2090305@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      Thank you, Stefan, for review. I fixed up a little. Also I moved
      bug this changeset refers to from INTJDK to JDK: <a
        moz-do-not-send="true" id="key-val" rel="4693579"
        href="https://bugs.openjdk.java.net/browse/JDK-8027237">JDK-8027237</a>.<br>
      <br>
      I updated changeset:<br>
      - Added comments to <font size="1"> </font><font size="3">WB_StressVirtualSpaceResize

        method of whitebox<br>
        - Dropped changes of comment in virtualspace.cpp<br>
        - Extracted class in </font>ReadFromNoaccessArea.java file<br>
      <br>
      Updated webrev: <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.03/">http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.03/</a><br>
    </blockquote>
    <br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.03/src/share/vm/prims/whitebox.cpp.udiff.html">http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.03/src/share/vm/prims/whitebox.cpp.udiff.html</a><br>
    <br>
    The indentation in the HotSpot code is two spaces.<br>
    <br>
    Could you fix the weird indentation in:<br>
    <pre>+    if (! (UseCompressedOops && rhs.base() != NULL &&
+                                      (Universe::narrow_oop_base() != NULL) &&
+                                      Universe::narrow_oop_use_implicit_null_checks())) {

</pre>
    Can you separate 'UseCompressedOops && _base' in the output?<br>
    <pre>+        tty->print_cr("WB_ReadFromNoaccessArea method is useless:\n "
+                "\t\"UseCompressedOops && _base != NULL\" is %d\n"
+                "\t\"Universe::narrow_oop_base() != NULL\" is %d\n"
+                "\t\"Universe::narrow_oop_use_implicit_null_checks()\" is %d",
+                UseCompressedOops && rhs.base() != NULL,
+                Universe::narrow_oop_base() != NULL,
+                Universe::narrow_oop_use_implicit_null_checks());</pre>
    <br>
    <br>
    <br>
    There are a couple of places where you mix types of different sizes.
    Some examples:<br>
    <br>
    %d is for int, but you dereference a char*:<br>
    <br>
    +    tty->print_cr("*(vs.low_boundary() - rhs.noaccess_prefix() /
    2 ) = %d", <br>
    +            *(vs.low_boundary() - rhs.noaccess_prefix() / 2 ));<br>
    <br>
    <br>
    magnitude is a jlong (8 bytes) and os::random() returns a long,
    which is platform dependent:<br>
    <br>
    +        x = (os::random() % (2 * magnitude)) - magnitude;<br>
    <br>
    <br>
    %d is int, but seed is long:<br>
    <br>
    +    tty->print_cr("Random seed is %d", seed);<br>
    <br>
    <br>
    Could you change x to be a size_t and widen the types where
    necessary, instead of shrinking them. For example:<br>
    <pre><span class="new">+        if (x < 0 && (((long) vs.committed_size()) + x) < 0) {
</span>
</pre>
    Mixing int (%d) and long.<br>
    <pre>+    tty->print_cr("reservedSpaceSize=%d, magnitude=%d, iterations=%d", 
+            (long) reservedSpaceSize, (long) magnitude, (long) iterations);

</pre>
    Could you move the declaration of 'long x' into the for loop?<br>
    <br>
    +    for (long x, i = 0; i < iterations; i++ ) {<br>
    +        <br>
    <br>
    Theres a spurious semicolon:<br>
    <pre>+        } else {
+            vs.shrink_by(-x);
+        };

</pre>
    thanks,<br>
    StefanK<br>
    <br>
    <blockquote cite="mid:526927FB.2090305@oracle.com" type="cite"> <br>
      <div class="moz-cite-prefix">On 10/20/2013 12:54 AM, <a
          moz-do-not-send="true" class="moz-txt-link-abbreviated"
          href="mailto:aleksey.timofeev@oracle.com">aleksey.timofeev@oracle.com</a>
        wrote:<br>
      </div>
      <blockquote cite="mid:5262F18B.301@oracle.com" type="cite">Hello.
        <br>
        <br>
        Probably, I didn't get responses for my previous letter because
        I sent it with wrong subject. <br>
        <br>
        Could somebody review my changeset containing new tests against
        ReservedSpace and VirtualSpace classes? Please find webrev here:
        <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.02/">http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.02/</a>
        <a moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.02/"><http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.02/></a>.
        Apart from feedback commit sponsorship wanted. <br>
        <br>
      </blockquote>
      <br>
      <pre class="moz-signature" cols="72">-- 
Best regards, Aleksey Timofeev.</pre>
    </blockquote>
    <br>
  </body>
</html>