<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>