[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(XS): 8038422: CDS test failed: assert((size % os::vm_allocation_granularity())
From:       David Holmes <david.holmes () oracle ! com>
Date:       2014-05-24 0:11:50
Message-ID: 537FE3C6.4030208 () oracle ! com
[Download RAW message or body]

Hi Yumin,

I think vm_allocation_granularity >= vm_page_size by definition, so I 
don't think using max of the two really makes sense. The fact these two 
values only differ on Windows just reinforces my concern as to whether 
all the code that uses one of these values really appreciates the 
difference between them (eg reserve with allocation granularity, and 
commit with page size).

Anyway it sounds like you have the correct fix here. My concern is 
really with other code.

Cheers,
David

On 24/05/2014 3:12 AM, Yumin Qi wrote:
> David,
>    Thanks for the review.
>    For all other platforms than Windows,  vm_allocation_granularity is
> same as vm_page_size.
>
>    From :
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa366887(v=vs.85).aspx
>
> /lpAddress/[in, optional]
>
>     The starting address of the region to allocate. If the memory is
>     being reserved, the specified address is rounded down to the nearest
>     multiple of the allocation granularity. If the memory is already
>     reserved and is being committed, the address is rounded down to the
>     next page boundary. To determine the size of a page and the
>     allocation granularity on the host computer, use the*GetSystemInfo*
>     <http://msdn.microsoft.com/en-us/library/windows/desktop/ms724381%28v=vs.85%29.aspx>function.
>     If this parameter is*NULL*, the system determines where to allocate
>     the region.
>
>   In case VirtualAlloc used to reserve memory,  we should align the
> reserve size aligned with dwAllocationGranularity; in case commit
> memory, we should use page size. I have seen some discussions and one
> solution is choose the max of the two.
>
>   We need to tell reserve/commit case for using these two properties. In
> case CDS (or other case reserve/commit combined),  the right way is
> choose max of the two I think.
>   My change can prevent reserve space rounded down to nearest multiple
> of page sizes --- which should be multiple of allocation granularity.
>
>    Maybe choose the max of the two is better solution, what do you
> think? This way, we have a uniform on all platforms.
>
> Thanks
> Yumin
>
> On 5/23/2014 4:11 AM, David Holmes wrote:
>> Hi Yumin,
>>
>> The change seems simple enough but it does make me wonder if these two
>> functions are being used correctly elsewhere. This isn't an aspect of
>> the VM I'm familiar with.
>>
>> David
>>
>> On 23/05/2014 5:51 PM, Yumin Qi wrote:
>>> Please review
>>>
>>>    webrev: http://cr.openjdk.java.net/~minqi/8038422/
>>>    bug: https://bugs.openjdk.java.net/browse/JDK-8038422
>>>
>>>    Summary: In debug version, the assert is against
>>> os::vm_allocation_granularity(), but in initialization, we use
>>> os::vm_page_size() to align the allocation size. In windows,
>>> _vm_page_size and _vm_allocation_granularity may not be same.
>>>
>>>     _vm_page_size = si.dwPageSize;
>>>    _vm_allocation_granularity = si.dwAllocationGranularity;
>>>
>>> This lead to the assertion failed. We should align allocation size with
>>> _vm_allocation_granularity.
>>>
>>>    test: manual testing on Windows.(which is the error on)
>>>    JPRT
>>>    jtreg
>>>
>>>    Thanks
>>>    Yumin
>>>
>>>
>>>
>
[prev in list] [next in list] [prev in thread] [next in thread]