RFR: 8318447: Move NMT source code to own subdirectory

[Stefan Karlsson]

On Thu, 19 Oct 2023 20:06:50 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> I think that NMT is deserving of its own subdirectory. Can we do a review of the changes before I fix the merge conflicts?
> 
> 1. Moved all the nmt source code from services/ to nmt/
> 2. Renamed all the include statements and sorted them
> 3. Fixed the include guards

Changes requested by stefank (Reviewer).

Changes requested by stefank (Reviewer).

src/hotspot/share/gc/parallel/psParallelCompact.cpp line 33:

> 31: #include "code/codeCache.hpp"
> 32: #include "compiler/oopMap.hpp"
> 33: #include "gc/parallel/parMarkBitMap.inline.hpp"

This uses a case-sensitive sort, whereas I think that when we add includes without using a sorting tool we would do so in a case-insensitive manner. I'm not sure we should make this change here. (The same goes for similar cases in the rest of the patch)

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 89:

> 87: #include "runtime/safepointMechanism.hpp"
> 88: #include "runtime/vmThread.hpp"
> 89: #include "nmt/memTracker.hpp"

This included end up at the wrong place.

src/hotspot/share/nmt/memBaseline.cpp line 32:

> 30: #include "runtime/safepoint.hpp"
> 31: #include "nmt/memBaseline.hpp"
> 32: #include "nmt/memTracker.hpp"

Sort order

src/hotspot/share/nmt/nmtPreInit.hpp line 33:

> 31: #include "runtime/atomic.hpp"
> 32: #endif
> 33: #include "nmt/memTracker.hpp"

The ASSERT Section should be moved down to after the main include block, or even better just include it without the guard. Might be nice to fix this while you are moving around the includes.

src/hotspot/share/nmt/threadStackTracker.cpp line 31:

> 29: #include "nmt/memTracker.hpp"
> 30: #include "nmt/threadStackTracker.hpp"
> 31: #include "nmt/virtualMemoryTracker.hpp"

Sort order

src/hotspot/share/nmt/virtualMemoryTracker.hpp line 33:

> 31: #include "nmt/allocationSite.hpp"
> 32: #include "nmt/nmtCommon.hpp"
> 33: #include "utilities/linkedlist.hpp"

This file didn't get an update to the include guard

src/hotspot/share/services/mallocTracker.inline.hpp line 30:

> 28: 
> 29: #include "services/mallocLimit.hpp"
> 30: #include "services/mallocTracker.hpp"

Missing include guard rename

src/hotspot/share/services/nmtPreInit.hpp line 33:

> 31: #include "runtime/atomic.hpp"
> 32: #endif
> 33: #include "services/memTracker.hpp"

Missing include guard rename

test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp line 31:

> 29: #include "utilities/debug.hpp"
> 30: #include "utilities/ostream.hpp"
> 31: 

You make this change in some files but not all. Maybe revert this unrelated change?

test/hotspot/gtest/nmt/test_nmt_cornercases.cpp line 26:

> 24: 
> 25: #include "precompiled.hpp"
> 26: 

You make this change in some files but not all. Maybe revert this unrelated change?

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

PR Review: https://git.openjdk.org/jdk/pull/16276#pullrequestreview-1689312655
PR Review: https://git.openjdk.org/jdk/pull/16276#pullrequestreview-1689327479
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366529454
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366521538
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366522881
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366524596
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366524886
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366525477
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366530660
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366530318
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366526935
PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1366527038