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

[Johan Sjölen]

On Sun, 19 Feb 2023 10:02:39 GMT, Thomas Stuefe <stuefe 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

Hi,

is the only cost to this the 148-95=53 additional LoC and an additional pointer indirection?

Have I understood it as this being the old way:


     +-------+---------+-------------------------------+    +-------+--------+-------------------------------+
     |       |         |                               |    |       |        |                               |
  +--| next  |  size   |        payload                |    | next  |  size  |            payload            |
  |  +--+----+---------+-------------------------------+    +-+-----+--------+-------------------------------+
  |     |                                                     |
  |     |                                                     |
  |     |                                                     |
  |     +-----------------------------------------------------+
  |
  |
  |
  |
  | +-------+---------+---------+-------+--------+--------+
  | |       |         |         |       |        |        |
  +-+---    |         |         |       |        |        |
    |       |         |         |       |        |        |
    +-------+---------+---------+-------+--------+--------+
                      _entries  



And this being the new way:


     +-------+---------+------+      +-------+--------+-------+
     |       |         | pay  |      |       |        | pay   |
  +--| next  |  size   | load |      | next  |  size  | load  |
  |  +--+----+---------+--+---+      +-++----+--------+--+----+
  |     |                 |             |                |
  |     |  +--------------+-------+     |        +-------+-------+
  |     |  |                      |     |        |               |
  |     |  |     payload          |     |        |    payload    |
  |     |  +----------------------+     |        +---------------+
  |     +-------------------------------+
  | +-------+---------+---------+-------+--------+--------+
  | |       |         |         |       |        |        |
  +-+---    |         |         |       |        |        |
    |       |         |         |       |        |        |
    +-------+---------+---------+-------+--------+--------+
                      _entries


(this might render better on Github than it does in e-mail, FYI)

And we always only handle the `payload` pointers, so that's why we can delete the table if NMT is turned off. OK, that sounds great to me. I think not having an implicit payload hanging off of the `NMTPreInitAllocation` is simpler anyway, less to remember.

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

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