RFR(S) 8015255: NPG: Don't waste fragment at the end of a VirtualSpaceNode before retiring it.

[Mikael Gerdin]

Jon,

Thanks for reviewing, see my reply inline:

On 10/17/2013 06:09 PM, Jon Masamitsu wrote:
> Looks good.  Some questions below.  No blockers to the integration.
>
>> 1120   for (int i = (int)MediumIndex; i >= (int)ZeroIndex; --i) {
>> 1121     ChunkIndex index = (ChunkIndex)i;
>
> What do you think about code like this that uses ChunkIndex as the
> for loop counter.
>
>    for (ChunkIndex i = Zer Index; i < NumberOfFreeLists; i =
> next_chunk_index(i))
>
> I'm asking because I like the way it looks but it's not perfect.

I had a go at doing what you suggested, but I didn't like it.
I should probably change it to be similar to how you suggest, but I ran 
into an issue with my prev_chunk_index where I didn't want to return the 
value "-1", but "-1" is actually the loop exit condition.

>
>> 3674     // The chunk sizes must be multiples of eachother, or this
>> will fail
>> 3675     STATIC_ASSERT(MediumChunk % SmallChunk == 0);
>> 3676     STATIC_ASSERT(SmallChunk % SpecializedChunk == 0);
>
> The above is a requirement now?   I've never thought of it as a
> requirement.  It just
> happened to be the case.  I'm not opposed to it, just wondering.

It's not a requirement per se, but the retire code and the test code 
relies on this fact currently, so I want to make sure that it gets 
caught if it changes.

>
> 3724       vsn.get_chunk_vs(MediumChunk + SpecializedChunk); //
> Humongous chunks will be aligned up to MediumChunk + SpecializedChunk
>
> Should the chunk size beMediumChunk + SpecializedChunk  / 2
> to test the alignment up?

get_chunk_vs doesn't do the alignment up. I pass in a pre-aligned value 
which should be equivalent to
align_size_up(MediumChunk + 1, SpecializedChunk)

/Mikael

>
> Jon
>
>
>
>
>
>
> On 10/17/2013 5:56 AM, Mikael Gerdin wrote:
>> Hi all,
>>
>> Please review this change to fix needless memory waste in Metaspace.
>>
>> Description of problem:
>> When receiving an allocation request which cannot be satisfied with the
>> current VirtualSpaceNode the remaining committed memory in the current
>> node is
>> wasted when a new node is created and the allocation is satisfied from
>> the new
>> node.
>>
>> Suggested fix:
>> Before creating a new VirtualSpaceNode, "retire" the current node by
>> allocating chunks of decreasing size from the committed memory present
>> in the
>> current node.
>> To ensure that allocating chunks at the end of a node uses all committed
>> memory I've increased the size alignment for humongous chunks to the
>> smallest
>> chunk size. I don't foresee this increasing memory waste since
>> previously the
>> memory difference between 2-word-aligned sizes and smallest chunk-aligned
>> would go unused since no chunk would fit there.
>>
>> Testing:
>> Added new unit tests for VirtualSpaceNode and the retire functionality.
>> JPRT run (including the new unit tests)
>>
>> Webrev: http://cr.openjdk.java.net/~mgerdin/8015255/webrev.0/
>> Buglink: https://bugs.openjdk.java.net/browse/JDK-8015255
>>
>> /Mikael
>