[PATCH] JDK-8176571: Fine bitmaps should be allocated as belonging to mtGC

[Kim Barrett]

> On May 22, 2017, at 11:23 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi Milan,
> 
>   first, thanks for your contribution! :)
> 
> Some procedural points: we would need you to sign a contributor
> agreement if you are not covered yet in some way to be able to take
> your contributions.
> 
> See the FAQ [1] and the form itself [2].
> 
> On Mon, 2017-05-22 at 06:35 +0000, Milan Mimica wrote:
>> Just fixing the subject
>> 
>> From: hotspot-gc-dev <hotspot-gc-dev-bounces at openjdk.java.net> on
>> behalf of Milan Mimica <Milan.Mimica at infobip.com>
>> Sent: Sunday, May 21, 2017 13:12
>> To: hotspot-gc-dev at openjdk.java.net
>> Subject: RFR(XS) JDK-8176571:
>>  
>> Hello
>> 
>> Here is my attempt to fix https://bugs.openjdk.java.net/browse/JDK-81
>> 76571. The same applies to jdk9 and 10.
>> It changes the NMT category of CHeapBitMap which is used only in G1
>> code. Given that the class is defined in "share" scope, and it may be
>> used elsewhere in the future, perhaps it should be made made a
>> template? That will add more boilerplate code to g1 sources.
> 
> It is a bit awkward to have a generic utility class to always report
> everything as belonging to GC :-)
> 
> We actually discussed this a little internally, and think that adding a
> template parameter would pull out a lot of code from the cpp files into
> the header files. As we kind of try to hide implementation as much as
> possible, such a change would work against that goal.

Not necessarily. CHeapBitMap could be split into a base class with
allocation functions taking a MEMFLAGS argument, and a MEMFLAGS
templated wrapper with allocation functions that pass that template
parameter's value to the base class allocation function.

I haven't thought much about whether that's better than the other
option (below), of an additional field in CHeapBitMap, which is the
MEMFLAGS to use.  I can see arguments for and against both.

> There is another option, having an additional field in CHeapBitmap that
> indicates from what pool the memory should be assigned to.
> From a memory POV that additional (4-byte) member should be zero-cost
> for the remembered sets due to padding (see class PerRegionTable in
> heapRegionRemSet.cpp, immediately after the CHeapBitmap there is an
> int-sized member followed by a pointer).

I think any changes along these lines (either the template +
supporting runtime argument, or a new field in CHeapBitMap) runs afoul
of a different problem. There doesn't seem to be a good path into the
functionality provided by ArrayAllocator<> that has such a runtime
MEMFLAGS value, rather than a compile time (e.g. template parameter)
value. This is even though ultimately, down at the very bottom of the
call chain, the template parameter gets turned into a runtime value.
The lack of a runtime-only path for propagating that information would
need to be added.

> (All other CHeapBitmap are typically very large or very rare that it
> does not matter).

I dislike building such an assumption into a core utility of this kind.