RFR: JDK-8302820: Remove costs for NMTPreInit when NMT is off

Thomas Stuefe stuefe at openjdk.org
Sat Feb 25 07:02:03 UTC 2023


On Fri, 24 Feb 2023 19:47:48 GMT, Justin King <jcking at openjdk.org> wrote:

>> NMTPreInit has been brought into question lately (see [JDK-8299196](https://bugs.openjdk.org/browse/JDK-8299196) and [JDK-8301811](https://bugs.openjdk.org/browse/JDK-8301811)). The points of contention were costs paid when NMT is off, and complexity.
>> 
>> I believe NMTPreInit is vital (for reasons why see discussion under [8299196](https://bugs.openjdk.org/browse/JDK-8299196) ), and removing it would be a severe mistake. So let's address the cost problem.
>> 
>> NMTPreInit, in its current form, incurs costs post-init for lookup table lookup to identify pre-init allocations. Granted, this cost is already pretty low since the load factor of that table is small. But we can avoid that lookup completely by allocating pre-init blocks without malloc headers.
>> 
>> That has two advantages:
>> - costs for NMTPreInit for NMT=off are practically nil now: all that remains is querying NMT tracking level to see if we are pre- or post-init, and we need to do that anyway to see if NMT is switched on. That cost is not going away unless we get rid of NMT itself.
>> - We can delete the lookup table if NMT is off, since we don't need it nomore, and regain 63352 bytes of memory.
>> 
>> -----
>> 
>> I have done my best to come up with a good compromise between complexity, startup speed, and memory consumption. With a bit more complexity, penalties to startup speed could be even more reduced (e.g. by shepherding preallocation headers into their arena). 
>> 
>> But I'm between a rock and a hard place here: more complexity increases the chance of "its too complex, let's just remove it", which is a tiny bit stressful tbh. And the one point I feel strongly about is that getting rid of NMTPreInit would be a grave mistake. I also don't think this code needs more optimization.
>> 
>> (Please note that I enter vacation and won't be able to react promptly to reviews.)
>> 
>> ---
>> 
>> Tests: 
>> - Manually tested linux x64 and x86 (gtests with all NMT permutations; runtime/NMT)
>> - GHAs ongoing
>
> src/hotspot/share/services/nmtPreInit.cpp line 101:
> 
>> 99: 
>> 100: NMTPreInitAllocationTable::NMTPreInitAllocationTable() {
>> 101:   ::memset(_entries, 0, sizeof(_entries));
> 
> Hmm. Couldn't we do away with the array of pointers and just make it flat as an array of NMTPreInitAllocation? Then we could treat it as a map of malloc pointer to NMTPreInitAllocation? This would then allow us to just free the entire table, headers included, if NMT is off.

It is an open-addressing hashmap, so I could only inline the first layer of nodes. An early version of NMTPreInit had a closed-addressing map to keep all nodes as a flat array, but that limited the total number of possible pre-init allocations, and I did not want that. If some crazy person wants to call the JVM with a million arguments I won't deny him that.

Making only the first layer as a flat array would be possible, and - seeing how the load factor is so small - get rid of the lion's share of node allocations. But at the cost of more complexity (need to rewrite all node iterations) and more costs if the table is allocated (+~128K). Since complexity was one of the reasons stated to remove NMTPreInit, I try to avoid complexity as much as possible.

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

PR: https://git.openjdk.org/jdk/pull/12642


More information about the hotspot-runtime-dev mailing list