RFR (S): G1: Use ArrayAllocator for BitMaps

Tue Jun 18 07:41:17 PDT 2013

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.

> 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.

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
>>>>>>
>>>>
>>
>