[master] RFR: JDK-8307002: [Lilliput] [Metaspace] Make use of Klass alignment gaps [v5]

Fri May 5 06:06:44 UTC 2023

On Fri, 5 May 2023 05:51:07 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> https://github.com/openjdk/lilliput/pull/13 changed nKlass encoding by increasing the Klass alignment gap to 512. Works fine and gives us a nKlass-value-point-to-class ratio of about 1.5:1, which means we get close to the goal of representing each class with a single value of nKlass without changing Klass to a fixed-sized Layout. It has two disadvantages, though. The most obvious one - the memory waste caused by alignment shadows - is addressed by this issue. 
>> 
>> (The second disadvantage that I don't address in this issue is what John Rose called "naive overalignment", but it has effects on the patch, see below)
>> 
>> That waste matters little for normal programs: the average waste is 256 bytes per class, and if Lilliput saves 4 bytes per object, we break even if we create more than 64 objects per class on average. We usually do that.
>> 
>> But class-generation scenarios are a different matter. There, we often only have a single or very few objects per class, and many many more classes, so the numbers don't add up anymore. So we need a way to eliminate the gaps.
>> 
>> The simple solution is to save non-class metadata from the same class loader into these gaps. This works fine since much of our non-class metadata is very fine granular. Like sand into cracks, they settle into the gaps and fill them nicely. And it is not even complicated since we already have the machinery (the descendant of Coleen's original dark-matter-prevention-structure that we use to manage prematurely deallocated metaspace). 
>> 
>> ----
>> 
>> Numbers:
>> 
>> Memory usage is almost back to baseline now. Volkers insane lambda generation test from JDK-8302154, loading 1 million lambdas, -Xshare:off, shows a reduction of 1.74GB->1.32GB, very close to the original 1.31GB baseline:
>> 
>> (interesting because scenarios like these will be the worst case from a metaspace perspective - metaspace usage of normal apps is unexciting with or without Lilliput):
>> 
>> 
>> Lilliput unpatched:
>> Virtual space:                                    
>>   Non-class space:      832,00 MB reserved,     807,12 MB ( 97%) committed,  13 nodes.
>>       Class space:        1,00 GB reserved,     977,31 MB ( 95%) committed,  1 nodes.                                                     
>>              Both:        1,81 GB reserved,       1,74 GB ( 96%) committed. 
>> 
>> Lilliput patched:
>> Virtual space:                                    
>>   Non-class space:      384,00 MB reserved,     370,75 MB ( 97%) committed,  6 nodes.
>>       Class space:        1,00 GB reserved,     977,31 MB ( 95%) committed,  1 nodes.                                                     
>>              Both:        1,38 GB reserved,       1,32 GB ( 96%) committed. 
>> 
>> Stock VM:
>> Virtual space:                                    
>>   Non-class space:      832,00 MB reserved,     822,44 MB ( 99%) committed,  13 nodes.
>>       Class space:        1,00 GB reserved,     521,31 MB ( 51%) committed,  1 nodes.
>>              Both:        1,81 GB reserved,       1,31 GB ( 72%) committed. 
>> 
>> 
>> ----
>> 
>> Patch:
>> 
>> https://github.com/openjdk/lilliput/pull/13 introduced the notion that every MetaspaceArena has an alignment - instead, as in the stock VM - working with one global alignment. This patch reverts much of this code (makes it easier to merge too). Instead, it treats alignment as an allocation option. An arena can satisfy allocations with different alignment requirements and will do its best to salvage the alignment waste.
>> 
>> That is the core of the patch. The rest is fluff and tests.
>> 
>> ---
>> 
>> Notes:
>> 
>> Numbers are somewhat fluid: at the moment, we use a Klass alignment of 512 and a nKlass size of 21 bits. We will probably change nKlass width to 24 or 26 bits for Lilliputs first milestone.
>> 
>> And once this patch is in, the disadvantage of memory waste is eliminated, so we could increase the Klass alignment. Both 512 and 1K are good values: on average a Klass is between 400b...1K. A 1K alignment would spread out the nKlass value points more. Shift the nKlass-value-point-to-class ratio closer to 1:1.
>> 
>> The still unsolved problem is naive overalignment. We may change the encoding scheme of Klass*->nKlass to take that into account. But not in this patch. But this uncertainty is the reason why I opted for a new method `MetaspaceArena::allocate_for_klass` instead of just giving the existing `Metaspace::allocate` method an alignment parameter.
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix tests

I like it! I also like that you now preserve legacy behaviour when running with -UseCompactObjectHeaders :-) Change looks good, just a small nit.

src/hotspot/share/memory/metaspace/binList.hpp line 35:

> 33: #include "utilities/globalDefinitions.hpp"
> 34: 
> 35: 

stray newline?

-------------

Marked as reviewed by rkennke (Lead).

PR Review: https://git.openjdk.org/lilliput/pull/86#pullrequestreview-1414212042
PR Review Comment: https://git.openjdk.org/lilliput/pull/86#discussion_r1185724142