RFR: 8151436: Leaner ArrayAllocator and BitMaps
stefan.karlsson at oracle.com
Wed Mar 9 15:50:33 UTC 2016
On 2016-03-09 16:26, Per Liden wrote:
> On 2016-03-09 15:28, Stefan Karlsson wrote:
>> Hi all,
>> I found a bug in the ArrayAllocator code. ArrayAllocator::free
>> incorrectly calls should_use_malloc(size_for_malloc(length)) when the
>> call should be should_use_malloc(length).
>> There is an -XX:+ExecuteInternalVMTests that exercises this code path,
>> and try to find these kinds of bugs. The test actually manages to call
>> allocate_malloc and then tries to free the memory with free_mmap, but
>> the JVM doesn't crash. One reason why it doesn't crash is that
>> os::release_memory, which is used by free_mmap, fails to unmap the
>> malloced memory and we silently ignores that.
>> This patch:
>> 1) Adds an assert to ensure that the os::release_memory call succeeds.
>> This immediately shows the problem when we execute our internal vm
>> 2) Moves the implementation of should_use_malloc to the inline file, so
>> that it can reuse size_for_malloc.
>> 3) Fix the bug.
> Looks good.
>> On 2016-03-08 11:33, Stefan Karlsson wrote:
>>> Hi all,
>>> Please review this patch to remove the state variables from
>>> ArrayAllocator and make it an AllStatic class instead. The main
>>> motivation for this patch is to cleanup the BitMaps data structure and
>>> make the instances smaller.
>>> The patch builds on the fact that the current code invokes
>>> AllocateHeap with the default value AllocFailEnum:EXIT_OOM, and
>>> therefore never returns NULL. This means that use_malloc will never be
>>> reset and the allocation strategy used (malloc or mmap) can be
>>> determined by simply looking at the size input parameter. Instead of
>>> changing the code to use AllocateHeap with AllocFailEnum::RETURN_NULL,
>>> I chose to keep the current behavior so that we could cleanup and
>>> minimize the size of our BitMaps.
More information about the hotspot-dev