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

Gerard Ziemski gziemski at openjdk.org
Fri Jul 29 19:06:43 UTC 2022


On Mon, 25 Jul 2022 22:54:15 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 two additional commits since the last revision:
> 
>  - moved os::{print_active_locale, print_user_info} to os_posix.cpp
>  - Fixed os.hpp comments per @dholmes-ora review

Looks good, however, I would prefer not to have to provide the default implementations for `register_code_area()` or `resolve_function_descriptor()`

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

> 33: 
> 34: // Below are inline functions that are rarely implemented by the platforms.
> 35: // Provide default emptyy implementation.

Typo: emptyy --> empty

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

> 53:   return true;
> 54: }
> 55: #endif

We could use some custom define, similar to `HAVE_FUNCTION_DESCRIPTORS` and remove this default impl just like with `resolve_function_descriptor()`?

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

> 58: inline void* os::resolve_function_descriptor(void* p) {
> 59:   return NULL;
> 60: }

This is unnecessary and could be removed if we did:


+#ifdef HAVE_FUNCTION_DESCRIPTORS
   // Used only on PPC.
   inline static void* resolve_function_descriptor(void* p);
+#endif


in `os.hpp`

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

Changes requested by gziemski (Committer).

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


More information about the hotspot-dev mailing list