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

[Ioi Lam]

On Tue, 2 Aug 2022 07:12:33 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> 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
>> 
>> I think we should avoid creating non-portable os::xxx APIs and then call them from shared code. There were some calls to os::Posix::xxx from the shared code that I fixed in this PR.
>> 
>> If you really want to make a platform-specific call, you should just do this (sadly we have many such cases)
>> 
>> 
>> #ifdef LINUX
>> linux_blah();
>> #endif
>> 
>> 
>> Putting linux_blah() into the os class seems pointless.
>
>> 
>> I think we should avoid creating non-portable os::xxx APIs and then call them from shared code. There were some calls to os::Posix::xxx from the shared code that I fixed in this PR.
> 
> I don't think that's what I meant. I meant APIs that exist for all OSes but have different expressions on each OS. So the prototype should live in shared os. But I want to pull a different implementation for each OS. 
> 
> This only affects inline functions. If the different implementations live in os_xx.cpp it works automatically through link time resolution.
> 
> I see there are only a few examples for what I meant. Many inline functions return platform dependend values and those live in variables in shared code (e.g. os::vm_page_size(), ...). Others are just not inline (os.create_thread(), ...). Which leaves us with the likes of `zero_page_read_protected()` etc, and I see you put them into os.inline.hpp and there you include the OS specific headers too.
> 
> I guess this can mostly be avoided. E.g. there is zero reason why `zero_page_read_protected()` cannot be a static boolean variable in os. For future RFEs.

Thanks @tstuefe @gerard-ziemski @stefank @dholmes-ora for the review.
Passed build tiers 1 and 5.

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

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