RFR (S): G1: Use ArrayAllocator for BitMaps

Bengt Rutisson bengt.rutisson at oracle.com
Tue Jun 18 13:52:37 PDT 2013 

On 6/18/13 4:41 PM, Coleen Phillimore wrote:
>
> On 06/18/2013 10:31 AM, Bengt Rutisson wrote:
>>
>> Hi Coleen,
>>
>> Thanks for looking at this!
>>
>> On 6/18/13 3:21 PM, Coleen Phillimore wrote:
>>>
>>> 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?
>>
>> Now I'm confused.
>>
>> I'm fine with switching to ResourceObj, but I thought that was 
>> supposed to be a way of tagging objects that should be allocated in 
>> resource areas. Here is the comment in allocation.hpp that I am using 
>> as a reference:
>>
>> // For objects allocated in the resource area (see resourceArea.hpp).
>> // - ResourceObj
>> //
>> // For objects allocated in the C-heap (managed by: free & malloc).
>> // - CHeapObj
>> //
>> // For objects allocated on the stack.
>> // - StackObj
>> //
>> // For embedded objects.
>> // - ValueObj
>>
>>
>> Currently ArrayAllocator is always embedded in another object, so I 
>> figured that David's point was valid that it should be a ValueObj. 
>
> Oh, yes, you are right.  I didn't know this.   ValueObj is correct for 
> ArrayAllocator if they are embedded in other objects.   I got confused 
> by the class vs. what it did.

OK. Good. :)

>
>> But for the ResourceObj class there is this comment:
>>
>> //----------------------------------------------------------------------
>> // Base class for objects allocated in the resource area per default.
>> // Optionally, objects may be allocated on the C heap with
>> // new(ResourceObj::C_HEAP) Foo(...) or in an Arena with new (&arena)
>> // ResourceObj's can be allocated within other objects, but don't use
>> // new or delete (allocation_type is unknown).  If new is used to 
>> allocate,
>> // use delete to deallocate.
>>
>> Which, I guess says that a ResourceObj can be used for pretty much 
>> anything. If that is right, what is the point of having the 
>> ResourceObj class?
>
> It is useful because it describes the normal case, but it can deal 
> with exceptions.  We have a few exceptions.   I thought this was one 
> but it's not.   Sorry for the noise.

Thanks again for looking at this change!

Bengt

>
> Coleen
>
>>
>> Thanks,
>> Bengt
>>
>>>
>>> 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
>>>>>>>
>>>>>
>>>
>>
>

More information about the hotspot-dev mailing list