RFR (S): G1: Use ArrayAllocator for BitMaps

[Coleen Phillimore]

Bengt,
I had a look at this change also.   I would pick ResourceObj over 
ValueObj for the ArrayAllocator class.  ValueObj really is embedded and 
is used very consistently, and ResourceObj is usually used for mixed 
cases like this.   Is there some complication to using ResourceObj?

Thanks,
Coleen


On 06/18/2013 12:57 AM, David Holmes wrote:
> Reviewed.
>
> 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
>>>>
>>