RFR: 8265473: Move os::Linux to its own header file [v4]

[Thomas Stuefe]

On Sat, 9 Jul 2022 23:27:31 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Another step of moving unnecessary stuff outside of os.hpp
>> 
>> The `os::Linux` class is used only by the Linux-specific code in HotSpot. Therefore, it should be moved outside of os.hpp, which is used by platform-independent code.
>> 
>> I don't have a good name for the new header. `os_linux.hpp` would have been a good name, but that's already taken, so I am settling on os_linux.impl.hpp. Suggestions are welcome.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   renamed to os_linux_impl.hpp

Interesting about forward-declaring nested classes, did not know that is possible.

I think what you are trying to do - disentangling the interface and removing platform-dependent stuff - is really useful. I even think a lot of the platform-dependent stuff does not even need to appear in any header, local static functions in os_xxx.cpp would have been fine.

I propose a slightly alternative way for this though. How about making `os` a real namespace? That's what AllStatic tries to be anyway. It's only a class because namespaces did not exist when it was introduced.

In contrast to a class you can extend namespaces. Whereas a class must be always complete.

You could have:

os.hpp

namespace os {
  ..
  // Return the default page size.
  static int    vm_page_size();
  .. // many more
}


os_linux.hpp

namespace os {
  namespace Linux {
    ...
     void print_process_memory_info(outputStream* st);
     void print_system_memory_info(outputStream* st);
   ...
  }
}


Compilation units just needing the generic part can include `os.hpp` and be done with it. os_linux.cpp would include both os.hpp and os_linux.hpp. 

As a bonus, we can remove those awkward injections of class definitions into the middle of class os. The os_xxx.hpp headers then would become standard headers, includable on their own.

We would loose the ability to make os:: functions private in the interface. But that can be solved by moving them into their own header. Or really taking a good look - why would private members have to appear in a public interface?

I realize that this would be a bigger change than what you have planned, but I think that this way would be the standard way to organize an interface like this, easier to understand and to maintain. What do you think?

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

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