Review request: 8026391: The Metachunk header wastes memory - 8026392: Metachunks and Metablocks are using a too large alignment
jon.masamitsu at oracle.com
Mon Oct 14 15:46:45 PDT 2013
On 10/14/13 2:31 PM, Coleen Phillimore wrote:
> I think this looks good but I didn't review it completely. I had the
> opposite request as Jon though. I think "Metamem" is odd and if
> there's no reason to abbreviate something used only a couple places.
> Can you name the class Metamemory? And leave the file name
> metachunk? We have a lot of examples where the base class is not the
> same as the name of the file and the file is named after the class
> that the JVM uses outside the file (eg. growableArray.hpp).
I would go along with that except that Metablock is in the same file so
you have 2 VM visible types in the same file that is named after one
of them. I'll grant you the longer name Metamemory. I think it's a
little ugly. Metabase?
> Now I've reviewed it. I think the Metamemory refactoring.
> On 10/14/2013 04:40 PM, Jon Masamitsu wrote:
>> Comments only on the first patch.
>> Looks good. Couple of questions.
>> Why the change from _is_free to _is_marked_free?
>> 58 bool _is_free;
>> 105 DEBUG_ONLY(bool _is_marked_free;)
>> Is it to specifically distinguish it from the FreeChunk is_free()?
>> The "marked" in the name feels a little like GC when it
>> isn't GC related. Would "_is_tagged_free" make sense?
>> Did you consider changing the file names from Metachunk.* to
>> Metamem.*? The file contains the classes Metamem, Metablock,
>> and Metachunk. It sees more natural to me to have the file named
>> after the more general Metamem class. Then all the classes in
>> the file are Metamem (or derived from), yes?
>> On 10/14/13 4:58 AM, Stefan Karlsson wrote:
>>> Please, review these two patches to remove some of the wastages
>>> introduced in the metaspace.
>>> 8026391: The Metachunk header wastes memory
>>> There are a couple of fields in Metachunk that store already known
>>> information, like the beginning and the end of the Metachunk. I
>>> replaced those fields with code instead.
>>> To make this easier, I extracted some duplicated code in Metablock
>>> and Metachunk into a super class named Metamem. The extracted code
>>> is purely needed to allow these objects to be placed on the FreeList
>>> and in the BinaryTreeDictionary.
>>> 8026392: Metachunks and Metablocks are using a too large alignment
>>> The metaspace code was using ARENA_ALIGN to align the Metablocks and
>>> Metachunk header. This forced all allocations of metadata to be 16
>>> bytes aligned. This patch patch changes the restriction to be 8
>>> bytes instead, since that's the restriction needed for 64 bit types
>>> like long, double, pointers and klass pointers,
>>> Testing: Added unit test for Metachunk. JPRT (before splitting of
>>> the patches)
More information about the hotspot-dev