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

[Thomas Stuefe]

On Fri, 29 Jul 2022 20:31:42 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 typo

Very good work. Only small remarks/nitpickings.

One question though, how does it work if I want to use an API implemented on a per-platform base in shared code? E.g. `zero_page_read_protected()`? I include `os.hpp` to get the definition, but how do I get the implementation? I have to include the platform-specific inline header too? Still with the OS_HEADER macro?

Cheers, Thomas

src/hotspot/os/aix/os_aix.hpp line 180:

> 178:   static bool get_meminfo(meminfo_t* pmi);
> 179: 
> 180:   // PPC only

Since your are here, remove comment? There has not been an AIX on something other than power for ~25 years.

src/hotspot/os/aix/os_aix.inline.hpp line 37:

> 35: inline bool os::zero_page_read_protected() {
> 36:   return false;
> 37: }

I have the feeling many of these functions are really just global constants and that the coding could be simplified further in followup RFEs.

src/hotspot/os/linux/os_linux.cpp line 4492:

> 4490:   // perform the workaround
> 4491:   Linux::capture_initial_stack(JavaThread::stack_size_at_create());
> 4492:   Linux::workaround_expand_exec_shield_cs_limit();

You could move `workaround_expand_exec_shield_cs_limit()` into this file, declare it static, and remove all mentionings from headers. It now lives in os_linux_x86.cpp, but since it has to be wrapped with !zero !64bit already it is an odd misfit anyway. So its not much more "wrong" for it to live here.

src/hotspot/share/runtime/os.hpp line 92:

> 90: //   The Porting APIs declared as "inline" in os.hpp must be
> 91: //   implemented in one of the two .inline.hpp files, depending on whether
> 92: //   the feature is generic to the OS, or specific to a particular CPU

generic? Did you mean specific? E.g. "depending on whether the feature is specific to a certain OS or an OS+CPU combination" ?

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

> 35: // Provide default empty implementation.
> 36: 
> 37: #ifndef PLATFORM_PRINT_NATIVE_STACK

Could you rename this to HAVE_PLATFORM_PRINT_NATIVE_STACK?

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

Marked as reviewed by stuefe (Reviewer).

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