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

[Stefan Karlsson]

On 2013-10-29 15:21, aleksey.timofeev at oracle.com wrote:
> Hello.
>
> Updated fix: 
> http://cr.openjdk.java.net/~iignatyev/atimofeev/ResVirtSpaceTests/webrev.04/ 
> <http://cr.openjdk.java.net/%7Eiignatyev/atimofeev/ResVirtSpaceTests/webrev.04/>

Looks good.

thanks,
StefanK

>
> 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/60f0cdb1/attachment.htm>