On Fri, 20 Oct 2023 12:49:46 GMT, Johan Sjölen <jsjo...@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 > > Johan Sjölen has updated the pull request incrementally with one additional > commit since the last revision: > > Fix messed up include Changes requested by stefank (Reviewer). src/hotspot/share/nmt/memTracker.inline.hpp line 30: > 28: > 29: #include "nmt/mallocTracker.inline.hpp" > 30: #include "nmt/memTracker.hpp" Bonus points if you fix the include for this .inline.hpp file. Suggestion: #include "nmt/memTracker.hpp" #include "nmt/mallocTracker.inline.hpp" src/hotspot/share/nmt/nmtPreInit.hpp line 35: > 33: #include "utilities/macros.hpp" > 34: > 35: #ifdef ASSERT The blank line at 34 is not following the style for our conditional includes. Remove it, or better yet skip conventionalize the include of runtime/atomic.hpp since it just adds to noise to the file. ------------- PR Review: https://git.openjdk.org/jdk/pull/16276#pullrequestreview-1691967144 PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1368296670 PR Review Comment: https://git.openjdk.org/jdk/pull/16276#discussion_r1368299406
