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

Stefan Karlsson stefank at openjdk.org
Tue Aug 2 16:52:20 UTC 2022


On Tue, 2 Aug 2022 16:06:10 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
> 
>  - clarified comments about inline functions declared in os.hpp
>  - Merge branch 'master' into 8290840-refactor-the-os-class
>  - made workaround_expand_exec_shield_cs_limit() static; added PPC comments for os::resolve_function_descriptor()
>  - fixed whitespace
>  - clarify comment
>  - PLATFORM_PRINT_NATIVE_STACK -> HAVE_PLATFORM_PRINT_NATIVE_STACK
>  - simplified rarely-defined function os::resolve_function_descriptor()
>  - Made workaround_expand_exec_shield_cs_limit() a static function in os_linux.cpp
>  - fixed typo
>  - moved os::{print_active_locale, print_user_info} to os_posix.cpp
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/eae2b95f...b6e60a54

I love seeing the os files being cleaned up and restructured.

I started to look at the patch and found a significant number of nits that I think would be good to fix.

src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp line 37:

> 35: #include "nativeInst_zero.hpp"
> 36: #include "os_posix.hpp"
> 37: #include "os_linux.hpp"

Sort

src/hotspot/share/runtime/os.inline.hpp line 31:

> 29: 
> 30: #include OS_HEADER(os)
> 31: #include OS_HEADER_INLINE(os)

We usually only explicitly include .inline.hpp and don't and let that file bring in the corresponding .hpp file.

test/hotspot/gtest/runtime/test_os.cpp line 37:

> 35: #include "runtime/frame.inline.hpp"
> 36: #include "runtime/threads.hpp"
> 37: 

Remove blank line

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

Changes requested by stefank (Reviewer).

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


More information about the hotspot-dev mailing list