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

Ioi Lam iklam at openjdk.org
Fri Jul 29 20:31:45 UTC 2022


On Fri, 29 Jul 2022 18:51:29 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> 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
>
> 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`

After this PR, os.hpp no longer includes any os- or cpu-specific os_xxx.hpp files. I did this to avoid including large amount of os-specific declarations (such as os::Linux, which is about 400 lines) into generic source files, which aren't interested in these declarations.

As a result of this change, there's no way to let os.hpp know whether `HAVE_FUNCTION_DESCRIPTORS` is defined for the current OS+CPU. If we want to do that, we need to either:

- include os_xxx.hpp back in os.hpp (which I want to avoid)
- add new header files such as `os_<os>.defs.hpp`  `os_<os>_<cpu>.defs.hpp` to define  `HAVE_FUNCTION_DESCRIPTORS`  (this will result in lots of header files that usually have nothing inside)

Since there are only 4 functions that fall under this pattern (functions that are used in generic code but most platforms have dummy implementations), I think what I have is a good compromise

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

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


More information about the hotspot-dev mailing list