RFR: 8277990: NMT: Remove NMT shutdown capability [v2]

Aleksey Shipilev shade at openjdk.java.net
Tue Dec 7 13:49:21 UTC 2021


On Thu, 2 Dec 2021 18:23:07 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> NMT shutdown functionality is a remnant of its first implementation, which could consume excessive amount of memory, therefore, it needed capability to shut it self down to ensure health of JVM. This is no longer a case for current implementation.
>> 
>> After JDK-8277946, there is no longer a use, so it can be removed.
>> 
>> Test:
>> - [x] hotspot_nmt
>> - [x] tier1 with NMT on
>
> Zhengyu Gu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   tstuefe's comments

Very nice cleanup. Some minor suggestions below, feel free to ignore them.

src/hotspot/share/services/mallocTracker.cpp line 263:

> 261: 
> 262: #ifdef ASSERT
> 263:   if (level >= NMT_summary) {

Suggestion:

  if (level > NMT_off) {


This way, we don't care where `NMT_summary` is in the enum. We enter this code on all paths when NMT is enabled?

src/hotspot/share/services/memTracker.cpp line 129:

> 127:   // recursive calls in case NMT reporting itself crashes.
> 128:   if (Atomic::cmpxchg(&g_final_report_did_run, false, true) == false) {
> 129:     if (enabled()) {

Looks to me, you can do `enabled() && Atomic::cmpxchg(...)`, thus saving a tiny bit of CPU cycles when NMT is disabled?

src/hotspot/share/services/memTracker.hpp line 141:

> 139: 
> 140:   static inline bool enabled() {
> 141:     return _tracking_level >= NMT_summary;

Suggestion:

    return _tracking_level > NMT_off;


Same reason as above.

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

Marked as reviewed by shade (Reviewer).

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


More information about the hotspot-dev mailing list