RFR: NoBugIDYet: Refactoring os classes

David Holmes david.holmes at oracle.com
Wed Mar 13 03:33:10 PDT 2013


Hi Rickard,

An initial comment on your description here:

> There are three classes: osSolaris, osSolarisSparc and os.
> These are tried together through a couple of typedefs. osSolaris is typedefed to osCurrent and osSolarisSparc is typedefed to osCpu.
> Inside the os class we now typedef osCurrent to pd and osCpu to cpu.
> This way we can now access things that was previously accessed through os::Solaris::some_method like: os::pd::some_method. That helps if we want to write some code that would be common for a couple of platforms, using an api through pd like:
>
> doSomething() {
>   os::pd::setup();
>   do_something();
>   os::pd::close();
> }

I don't think the "pd" makes sense because you have to know what the 
"pd" actually is. In old terms an os::Solaris method can call another 
os::Solaris method because it knows it is there. If "pd" is common 
somehow (say pthreads functionality) then that commonality has to be 
named eg os::pthreads, because there might be another commonality 
related to some other aspect of functionality (perhaps Dtrace), so they 
can't both be called "pd".

Cheers,
David

On 13/03/2013 7:50 PM, Rickard Bäckman wrote:
> Hi all,
>
> I've spent some time on trying to do some changes to (in my opinion) clean up the os_ files.
>
> The change is broken up into 4 web revs to make the changes easier to spot since it involves new files, renaming of files, etc.
>
> Step 1)
>     * removes the os::Bsd, os::Linux, etc classes. They are replaced by classes osBsd, osLinux, osPosix (renamed in a later step), etc.
>     * The #include os_name.hpp inside the class definition is moved to before the definition of class os.
>     * Renaming users to the new osBsd, osSolaris, osWindows, osLinux, etc.
>     * Adds a typedef osCurrent pd; inside the os class.
>
> Step 2)
>     * Some methods existed in all or almost all os_<os>_<platform>.hpp, they were moved to os.hpp.
>
> Step 3)
>    * Removes os_<os>_<platform>.hpp and replaces it with a os_cpu.hpp. A new os_cpu.cpp contains the implementations of those methods.
>    * Changes #includes of os_<os>_<platform> into #include "os_cpu"
>    * Adds a typedef osCpu cpu; inside the os class.
>
> Step 4)
>    * Renames os_posix to os_xnix (xnix is *nix).
>    * Renames os_<os>.ext to os_family.ext
>    * Includes are changed from:
>
> #ifdef TARGET_OS_FAMILY_linux
>   # include "os_linux.hpp"
>   #endif
>   #ifdef TARGET_OS_FAMILY_solaris
>   # include "os_solaris.hpp"
>   #endif
>   #ifdef TARGET_OS_FAMILY_windows
>   # include "os_windows.hpp"
>   #endif
>   #ifdef TARGET_OS_FAMILY_bsd
>   # include "os_bsd.hpp"
>   #endif
>
>    to:
>
> #include "os_family.hpp"
>
>
> I'll try to summarize the final result the way it would look for solaris_sparc:
>
> There are three classes: osSolaris, osSolarisSparc and os.
> These are tried together through a couple of typedefs. osSolaris is typedefed to osCurrent and osSolarisSparc is typedefed to osCpu.
> Inside the os class we now typedef osCurrent to pd and osCpu to cpu.
> This way we can now access things that was previously accessed through os::Solaris::some_method like: os::pd::some_method. That helps if we want to write some code that would be common for a couple of platforms, using an api through pd like:
>
> doSomething() {
>    os::pd::setup();
>    do_something();
>    os::pd::close();
> }
>
> This should enable us to move code from platform specific code (thread creation, signal handling, etc) that look very similar (Linux, Bsd, Solaris) into something else that utilizes APIs through os::pd or os::cpu.
>
> it enables us to have platform specific includes in what os_family.hpp (used to be os_linux.hpp) for platform specific things like semaphores.
>
> To show the layout of the files:
>
> file: os_family.hpp
> class osSolaris {
> };
>
> typedef osSolaris osCurrent;
>
> file: os_cpu.hpp
> class osSolarisSparc {
> };
>
> typedef osSolarisSparc osCpu;
>
> file: os.hpp
> class os {
>   typedef osCpu cpu;
>   typedef osCurrent pd;
> };
>
> Webrevs:
> http://cr.openjdk.java.net/~rbackman/refactor/step1/webrev/
> http://cr.openjdk.java.net/~rbackman/refactor/step2/webrev/
> http://cr.openjdk.java.net/~rbackman/refactor/step3/webrev/
> http://cr.openjdk.java.net/~rbackman/refactor/step4/webrev/
>
> Thanks
> /R
>


More information about the hotspot-dev mailing list