RFR (M): 7606282: tests on ReservedSpace/VirtualSpace

[aleksey.timofeev at oracle.com]

Hello.

Updated fix: 
http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.04/ 
<http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.04/>

I recall that I moved bug to JDK and now it's id is JDK-8027237 
<https://bugs.openjdk.java.net/browse/JDK-8027237>.

Thanks.

On 10/25/2013 03:48 PM, Stefan Karlsson wrote:
> Hi Aleksey,
>
> Adding hotspot-runtime-dev, since this is adding tests to test/runtime.
>
> Inlined:
>
> On 2013-10-24 16:00, aleksey.timofeev at oracle.com wrote:
>> Thank you, Stefan, for review. I fixed up a little. Also I moved bug 
>> this changeset refers to from INTJDK to JDK: JDK-8027237 
>> <https://bugs.openjdk.java.net/browse/JDK-8027237>.
>>
>> I updated changeset:
>> - Added comments to WB_StressVirtualSpaceResize method of whitebox
>> - Dropped changes of comment in virtualspace.cpp
>> - Extracted class in ReadFromNoaccessArea.java file
>>
>> Updated webrev: 
>> http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.03/ 
>> <http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.03/>
>
> http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.03/src/share/vm/prims/whitebox.cpp.udiff.html
>
> The indentation in the HotSpot code is two spaces.
>
> Could you fix the weird indentation in:
> +    if (! (UseCompressedOops && rhs.base() != NULL &&
> +                                      (Universe::narrow_oop_base() != NULL) &&
> +                                      Universe::narrow_oop_use_implicit_null_checks())) {
>
> Can you separate 'UseCompressedOops && _base' in the output?
> +        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());
>
>
>
> There are a couple of places where you mix types of different sizes. 
> Some examples:
>
> %d is for int, but you dereference a char*:
>
> +    tty->print_cr("*(vs.low_boundary() - rhs.noaccess_prefix() / 2 ) 
> = %d",
> +            *(vs.low_boundary() - rhs.noaccess_prefix() / 2 ));
>
>
> magnitude is a jlong (8 bytes) and os::random() returns a long, which 
> is platform dependent:
>
> +        x = (os::random() % (2 * magnitude)) - magnitude;
>
>
> %d is int, but seed is long:
>
> +    tty->print_cr("Random seed is %d", seed);
>
>
> Could you change x to be a size_t and widen the types where necessary, 
> instead of shrinking them. For example:
> +        if (x < 0 && (((long) vs.committed_size()) + x) < 0) {
>
> Mixing int (%d) and long.
> +    tty->print_cr("reservedSpaceSize=%d, magnitude=%d, iterations=%d",
> +            (long) reservedSpaceSize, (long) magnitude, (long) iterations);
>
> Could you move the declaration of 'long x' into the for loop?
>
> +    for (long x, i = 0; i < iterations; i++ ) {
> +
>
> Theres a spurious semicolon:
> +        } else {
> +            vs.shrink_by(-x);
> +        };
>
> thanks,
> StefanK
>
>>
>> On 10/20/2013 12:54 AM, aleksey.timofeev at oracle.com wrote:
>>> Hello.
>>>
>>> Probably, I didn't get responses for my previous letter because I 
>>> sent it with wrong subject.
>>>
>>> Could somebody review my changeset containing new tests against 
>>> ReservedSpace and VirtualSpace classes? Please find webrev here: 
>>> http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.02/ 
>>> <http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.02/>. 
>>> Apart from feedback commit sponsorship wanted.
>>>
>>
>> -- 
>> Best regards, Aleksey Timofeev.
>

-- 
Best regards, Aleksey Timofeev.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20131029/cff23a07/attachment.htm>