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

[Stefan Karlsson]

On 2013-10-17 14:56, 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

Very nice.

Some comments:

Would it make more sense to move the call to 
retire_current_virtual_space() into create_new_virtual_space() or link_vs()?

I think some punctation is missing between the two first lines:
+  // If an allocation doesn't fit in the current node a new node is created
+  // allocate chunks out of the remaining committed space in this node
+  // to avoid wasting that memory.

I think you have an extra "the" in this sentence:
+  // This always adds up because the all the chunk sizes are multiples of
+  // the smallest chunk size.

The other usages of HumongousChunkGranularity have to be changed to 
smallest_chunk_size().

I had a hard time understanding this test:
+    { // 4 pages of VSN is committed, some is used by chunks

could you change this line:
+      const size_t page_chunks = MIN2(4 * (size_t)os::vm_page_size() / 
BytesPerWord, vsn_test_size_words);

to:
        const size_t page_chunks = 4 * (size_t)os::vm_page_size() / 
BytesPerWord;

and add an assert that 4 * (size_t)os::vm_page_size() is less than a 
MediumChunk (or less than MediumChunk + SmallChunk + SpecializedChunk)?

thanks,
StefanK

>
> /Mikael