RFR: JDK-8277822: Remove debug-only heap overrun checks in os::malloc and friends [v3]

[Thomas Stuefe]

On Tue, 18 Jan 2022 01:33:06 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - Merge branch 'master' into JDK-8277822-Remove-debug-only-heap-overrun-checks-in-os-malloc-and-friends
>>  - Merge branch 'master' into JDK-8277822-Remove-debug-only-heap-overrun-checks-in-os-malloc-and-friends
>>  - initialize newly malloced memory
>>  - JDK-8277822-Remove-debug-only-heap-overrun-checks-in-os-malloc-and-friends
>
> src/hotspot/share/runtime/globals.hpp line 546:
> 
>> 544:           range(0, 9)                                                       \
>> 545:                                                                             \
>> 546:   product(ccstr, NativeMemoryTracking, DEBUG_ONLY("summary") NOT_DEBUG("off"), \
> 
> Hi Thomas,
> 
> I'm a bit concerned about always enabling part of NMT in debug as that affects the bulk of our hotspot testing. What is the potential overhead from enabling NMT this way? We would need to run this patch through our CI to ensure it doesn't introduce any issues.
> 
> Cheers,
> David

Hi David,

I understand your concern. I have had this patch in our internal tests enabled for several months now, but am fine with you guys doing more tests.

My arguments for this:

1) using NMT in debug builds in place of the old hard wired fences should be less overhead, not more. NMT is quite frugal. NMT=summary does:
  - increase the counter per NMT category (basically atomic-incing a number in a static array)
  - add and populate a malloc header and a footer
That's it. We did the same thing before. The old code atomically increased two global counters (`inc_stat_counter`), then added a header and a footer. Only the old header/footer had been much larger than what NMT does. So we use less memory, not more, in debug.

2) Another argument for this is that NMT is important. It gets used by support, typically in stressful situations. I rather have it thoroughly tested as part of our internal tests. I am not sure that we have ironed out every single NMT issue. NMT feels very stable to me, but there may be lingering issues. But then we should fix them.

All that said, I'd be happy for you to give this patch a spin in your CI. I can hold off the push for a bit.

Cheers, Thomas

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

PR: https://git.openjdk.java.net/jdk/pull/6554