RFR: 8290840: Refactor the "os" class [v5]

[Stefan Karlsson]

On Tue, 2 Aug 2022 07:07:54 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Please see [JDK-8290840](https://bugs.openjdk.org/browse/JDK-8290840) for the detailed proposal.
>> 
>> The `os` class, declared in os.hpp, forms the major part of the HotSpot porting interface. Its structure has gradually deteriorated over the years as new ports are created and new APIs are added.
>> 
>> This RFE tries to address the following:
>> 
>> - Clearly specify where a porting API should be declared and defined among the various `os*.cpp` and `os*.hpp` files.
>> - Avoid the inappropriate inclusion of OS-specific APIs (such as the `os::Linux class`) by platform-independent source files.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed whitespace

src/hotspot/os/posix/os_posix.cpp line 52:

> 50: #include "utilities/macros.hpp"
> 51: #include "utilities/vmError.hpp"
> 52: 

Other files don't add a blank line before the conditional includes.

src/hotspot/os/posix/os_posix.hpp line 29:

> 27: 
> 28: #include "runtime/os.hpp"
> 29: #include <errno.h>

We usually add blank lines before system includes

src/hotspot/os/posix/os_posix.inline.hpp line 30:

> 28: #include "runtime/mutex.hpp"
> 29: #include "runtime/os.hpp"
> 30: #include "os_posix.hpp"

The .hpp file of an .inline.hpp file should be included as the first include. The comment above was an indication that we previously didn't follow that rule, because of how we sometimes structure platform includes. But it seems like you are changing this now, and I hope that we now can follow that rule and change the code to:


#include "os_posix.hpp"

#include "runtime/mutex.hpp"
#include "runtime/os.hpp"

src/hotspot/os/posix/perfMemory_posix.cpp line 40:

> 38: #include "services/memTracker.hpp"
> 39: #include "utilities/exceptions.hpp"
> 40: 

Remove blank line

src/hotspot/os/posix/perfMemory_posix.cpp line 44:

> 42: #include "os_linux.hpp"
> 43: #endif
> 44: 

It feels "wrong" that the posix files include linux files. Maybe that's something that future restructuring of code could fix.

src/hotspot/os/posix/safefetch_static_posix.cpp line 31:

> 29: #include "runtime/safefetch.hpp"
> 30: #include "utilities/globalDefinitions.hpp"
> 31: #include "os_posix.hpp"

Not sorted include

src/hotspot/os/posix/semaphore_posix.cpp line 28:

> 26: #ifndef __APPLE__
> 27: #include "runtime/os.hpp"
> 28: #include "os_posix.hpp"

Sort

src/hotspot/os/posix/signals_posix.cpp line 38:

> 36: #include "runtime/javaThread.hpp"
> 37: #include "runtime/os.hpp"
> 38: #include "os_posix.hpp"

Sort

src/hotspot/os/posix/vmError_posix.cpp line 30:

> 28: #include "runtime/javaThread.hpp"
> 29: #include "runtime/os.hpp"
> 30: #include "os_posix.hpp"

Sort

src/hotspot/os/windows/os_windows.inline.hpp line 33:

> 31: #include "runtime/mutex.hpp"
> 32: #include "runtime/os.hpp"
> 33: #include "os_windows.hpp"

Sort

src/hotspot/os/windows/pdh_interface.cpp line 27:

> 25: #include "precompiled.hpp"
> 26: #include "pdh_interface.hpp"
> 27: #include "os_windows.hpp"

Sort

src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp line 37:

> 35: #include "memory/allocation.inline.hpp"
> 36: #include "os_posix.hpp"
> 37: #include "os_linux.hpp"

Sort

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

PR: https://git.openjdk.org/jdk/pull/9600