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

[David Holmes]

On Tue, 18 Jan 2022 06:06:46 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> 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

Thanks for the additional info Thomas. I will run it through our CI and report back.

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

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