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