RFR: 8241141: Restructure humongous object allocation in G1

Thomas Schatzl thomas.schatzl at oracle.com
Wed Apr 8 08:42:16 UTC 2020


Hi,

On 07.04.20 16:02, Stefan Johansson wrote:
> New webrevs incorporating the new suggested search from Kim:
> Full: http://cr.openjdk.java.net/~sjohanss/8241141/02/
> Inc: http://cr.openjdk.java.net/~sjohanss/8241141/01-02/
> 

- could you please remove the newline after the return parameter in 
g1CollectedHeap.cpp:211 while we are touching this code?

- in heapRegionManager.hpp I would like to ask to make the parameter 
name indicating the number of regions uniform. I.e. the change adds 
"num" and "regions" to the existing "num_regions" (and it also sometimes 
uses num_regions) for this kind of parameter for no apparent reason.

Looks good otherwise.

[...]
>>>>> Not part of this change, but I found the name of
>>>>> HeapRegionManager::is_available() pretty confusing, especially as used
>>>>> in expand_on_preferred_node.  The "available" nomenclature generally
>>>>> seems confusing to me; why isn't it "committed", since that seems to
>>>>> be what it means.

There is a distinction between available (for allocation), i.e. visible 
to the higher layer (e.g. G1CollectedHeap) and committed, which means 
that its memory is committed from the OS.

Consider large pages where page size > region size. While G1 may have 
logically given back some regions in a page to the OS (i.e. these are 
"unavailable"), G1 might not have been able to actually "uncommit" (give 
back to the OS) the page and its corresponding regions just yet.

G1 tracks the difference for various reasons. The use of a second bitmap 
so that there may be invalid/nonsensical combinations might occur might 
not have been optimal :)

I would guess the concurrent uncommit may want to add another state to 
that, i.e. some "in the process of being uncommitted".

>>>> I kind of agree, but if we move forward with concurrent uncommit 
>>>> this might change to really mean available rather than committed. We 
>>>> probably want to clear the available map before really uncommitting 
>>>> the regions.
>>>
>>> OK, so “committed” might not be the right term, but “available” 
>>> doesn’t have the right connotation either, at least for me.  I don’t have a better 
>>> suggestion just now though.
>> Yes, we could probably come up with something better.

I am open to naming improvements :)

Thanks,
   Thomas



More information about the hotspot-gc-dev mailing list