RFR (S): G1: Use ArrayAllocator for BitMaps

[Bengt Rutisson]

On 6/18/13 6:57 AM, David Holmes wrote:
> Reviewed.
Thanks!
Bengt

>
> Thanks.
>
> David
>
> On 18/06/2013 2:50 PM, Bengt Rutisson wrote:
>> On 6/18/13 5:34 AM, David Holmes wrote:
>>> Seems reasonable to me.
>>
>> Thanks, David!
>>
>> BTW, I am not sure if you reviewed the whole change or just commented on
>> the StackObj vs. ValueObj issue. Can I list you as a reviewer of the
>> change?
>>
>> Thanks,
>> Bengt
>>
>>>
>>> Thanks,
>>> David
>>>
>>> On 17/06/2013 4: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
>>>>
>>