RFR (S): G1: Use ArrayAllocator for BitMaps

[Bengt Rutisson]

On 6/18/13 9:41 PM, John Cuthbertson wrote:
> Hi Bengt,
>
> The latest webrev looks good to me. Sorry for being late.

Thanks, John!

Bengt

>
> JohnC
>
> On 6/16/2013 11:17 PM, Bengt Rutisson wrote:
>>
>> Hi David,
>>
>> On 6/17/13 5:23 AM, David Holmes wrote:
>>> Hi Bengt,
>>>
>>> I don't think it makes sense to embed a StackObj into a ValueObj! 
>>> But then why is ArrayAllocator a StackObj? The only existing use of 
>>> it I can see also embeds it! So seems to me it should be a ValueObj 
>>> in the first place.
>>
>> Good point. I changed ArrayAllocator to be a ValueObj instead. New 
>> webrev here:
>> http://cr.openjdk.java.net/~brutisso/8016556/webrev.01/
>>
>> Here is the diff compared to the last version:
>>
>> diff --git a/src/share/vm/memory/allocation.hpp 
>> b/src/share/vm/memory/allocation.hpp
>> --- a/src/share/vm/memory/allocation.hpp
>> +++ b/src/share/vm/memory/allocation.hpp
>> @@ -665,7 +665,7 @@
>>  // is set so that we always use malloc except for Solaris where we 
>> set the
>>  // limit to get mapped memory.
>>  template <class E, MEMFLAGS F>
>> -class ArrayAllocator : public StackObj {
>> +class ArrayAllocator VALUE_OBJ_CLASS_SPEC {
>>    char* _addr;
>>    bool _use_malloc;
>>    size_t _size;
>>
>>
>> Thanks for looking at this!
>> Bengt
>>
>>
>>>
>>> David
>>>
>>> On 14/06/2013 12:53 AM, Bengt Rutisson wrote:
>>>>
>>>> Hi everyone,
>>>>
>>>> Could I have a couple of review for this small change?
>>>> http://cr.openjdk.java.net/~brutisso/8016556/webrev.00/
>>>>
>>>> Sending this request to the broad hotspot dev mailing list since it
>>>> touches code in vm/utilities.
>>>>
>>>> Background:
>>>>
>>>> In the constructor for the ConcurrentMark class in G1 we set up one 
>>>> bit
>>>> map per worker thread:
>>>>
>>>>    for (uint i = 0; i < _max_worker_id; ++i) {
>>>> ...
>>>>      _count_card_bitmaps[i] = BitMap(card_bm_size, false);
>>>>      ...
>>>>    }
>>>>
>>>> Each of these bitmaps are malloced, which means that the amount of
>>>> C-heap we require grows with the number of GC worker threads we 
>>>> have. On
>>>> a machine with many CPUs we get many worker threads by default. For
>>>> example, on scaaa007 I get 79 GC worker threads. The size of the 
>>>> bitmaps
>>>> also scale with the heap size. Since this large machine has a lot of
>>>> memory we get a large default heap as well.
>>>>
>>>> Here is the output from just running java -version with G1 on 
>>>> scaaa007:
>>>>
>>>> $ java -d64 -XX:+UseG1GC -XX:+PrintMallocStatistics -version
>>>> java version "1.8.0-ea-fastdebug"
>>>> Java(TM) SE Runtime Environment (build 1.8.0-ea-fastdebug-b92)
>>>> Java HotSpot(TM) 64-Bit Server VM (build 25.0-b34-fastdebug, mixed 
>>>> mode)
>>>> allocation stats: 35199 mallocs (221MB), 13989 frees (0MB), 35MB resrc
>>>>
>>>> We malloc 221MB just by starting the VM. Most of the large allocations
>>>> are due to the BitMap allocations. My patch changes the BitMap
>>>> allocations to use the ArrayAllocator instead. That class uses mmap on
>>>> Solaris if the size is larger than 64K.
>>>>
>>>> With this patch the output looks like this:
>>>>
>>>> $ java -d64 -XX:+UseG1GC -XX:+PrintMallocStatistics -version
>>>> java version "1.8.0-ea-fastdebug"
>>>> Java(TM) SE Runtime Environment (build 1.8.0-ea-fastdebug-b93)
>>>> Java HotSpot(TM) 64-Bit Server VM (build
>>>> 25.0-b34-internal-201306130943.brutisso.hs-gc-g1-mmap-fastdebug, mixed
>>>> mode)
>>>> allocation stats: 35217 mallocs (31MB), 14031 frees (0MB), 35MB resrc
>>>>
>>>> We are down to 31MB.
>>>>
>>>> Note that the ArrayAllocator only has this effect on Solaris machines.
>>>> Also note that I have not reduced the total amount of memory, just 
>>>> moved
>>>> it from the C-heap to mapped memory.
>>>>
>>>> One complication with the fix is that the BitMap data structures get
>>>> copied around quite a bit. The copies are shallow copies, so we don't
>>>> risk re-doing the allocation. But since I am now embedding an
>>>> ArrayAllocator in the BitMap structure the destructor for copies of 
>>>> the
>>>> ArrayAllocator gets called every now and then. The BitMap explicitly
>>>> frees up the allocated memory when it thinks it is necessary. So, 
>>>> rather
>>>> than trying to refactor the code to avoid copying I made it 
>>>> optional to
>>>> free the allocated memory in the ArrayAllocator desctructor.
>>>>
>>>> I do think it would be good to review the BitMap code. It seems a bit
>>>> fishy that we pass around shallow copies. But I think my current 
>>>> change
>>>> keeps the same behavior as before.
>>>>
>>>> Thanks,
>>>> Bengt
>>
>