Code Review for Fix non-PCH build on Linux, Windows and MacOS X

Prepared by:simonis on Tue Feb 26 18:30:40 CET 2013
Contributed by:volker.simonis@gmail.com
Workspace:/share/software/Java/OpenJDK/hsx/hotspot-rt/hotspot
Compare against: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot
Compare against version:4204
Summary of changes: 204 lines changed: 79 ins; 78 del; 47 mod; 15468 unchg
Patch of changes: 8008959.patch
Bug id: Bug Database
Author comments:

Recent changes have broken the no-PCH build again.

The fix requires some bigger changes this time but before going into the details - PLEASE, PLEASE add a no-PCH build to your JPRT (or nightly build or whatever) system to catch such errors. On platforms which don't support PCH (i.e. Solaris) the normal build 'naturally' works and I would consider it 'good practice' for a project to be able to build without such an esoteric:) feature like precompiled headers. Actually this only means that all the dependencies of a compilation unit are explicitly recorded in that compilation unit (which I'd consider self-evident).

Tested 'product' and 'jvmg' builds on Linux/x86_64, Linux/x86_32, Windows/x86_64, Solaris/Sparc_64, and MacOS X/x86_64.

The following places were easy to fix (just include the appropriate ".inline.hpp" files):

src/share/vm/oops/symbol.hpp 

- the inline methods increment_refcount() and decrement_refcount() used Atomic inline functions (inc() and dec()) without including "runtime/atomic.inline.hpp". According to Coleen, the two methods are not performance critical and can be safly moved into "symbol.cpp".

src/share/vm/memory/allocation.inline.hpp

- uses Atomic inline functions (load() and store()) without including "runtime/atomic.inline.hpp"

src/os/windows/vm/decoder_windows.cpp

- uses Arguments::get_java_home() without including "runtime/arguments.hpp"

src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp
src/os_cpu/bsd_x86/vm/orderAccess_bsd_x86.inline.hpp
src/os_cpu/solaris_x86/vm/orderAccess_solaris_x86.inline.hpp
src/os_cpu/solaris_sparc/vm/orderAccess_solaris_sparc.inline.hpp
src/os_cpu/windows_x86/vm/orderAccess_windows_x86.inline.hpp 

- use the Atomic inline functions (load() and store()) without including "runtime/atomic.inline.hpp"

src/os_cpu/bsd_x86/vm/atomic_bsd_x86.inline.hpp
src/os_cpu/linux_x86/vm/atomic_linux_x86.inline.hpp
src/os_cpu/linux_zero/vm/atomic_linux_zero.inline.hpp
src/os_cpu/linux_sparc/vm/atomic_linux_sparc.inline.hpp
src/os_cpu/windows_x86/vm/atomic_windows_x86.inline.hpp
src/os_cpu/bsd_zero/vm/atomic_bsd_zero.inline.hpp
src/os_cpu/solaris_sparc/vm/atomic_solaris_sparc.inline.hpp
src/os_cpu/solaris_x86/vm/atomic_solaris_x86.inline.hpp

- include "orderAccess_<os>_<cpu>.inline.hpp" without using any OrderAccess method. I removed this includes to avoid a cyclic dependency between "orderAccess_<os>_<cpu>.inline.hpp" and "atomic_<os>_<cpu>.inline.hpp" via "atomic.inline.hpp"

src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp
src/os_cpu/bsd_x86/vm/orderAccess_bsd_x86.inline.hpp
src/os_cpu/windows_x86/vm/orderAccess_windows_x86.inline.hpp 

- use os::is_MP() without including os.hpp

The following fix was a little more cumbersome:

src/os/bsd/vm/os_bsd.hpp
src/os/linux/vm/os_linux.hpp

- The methods "set_suspended()" and "clear_suspended()" use Atomic::cmpxchg without "atomic.inline.hpp" being included. Unfortunately "os_bsd.hpp" and "os_linux.hpp" are not self-contained files. They are themselves included into "os.hpp" right into the class definition of the "os" class. It is therefore not possible to simply include "atomic.inline.hpp" into "os_bsd.hpp" and "os_linux.hpp".

- The natural solution was to transfer the implementations of "set_suspended()" and "clear_suspended()" to the corresponding "os_bsd.inline.hpp" and "os_linux.inline.hpp" files. Unfortunately this is not trivial as well, because both methods use preprocessor defined constants from their enclosing class "SuspendResume" which are defined right before there definition and undefined right thereafter. They are therefore not available in "os_bsd.inline.hpp" and "os_linux.inline.hpp".

- The solution for the last problem is to change the mentioned preprocessor constants into an C++ enumeration which also results in a nicer overall code. The drawback is that all the users of the preprocessor constants have to be updated to use the new, qualified enum-values (i.e. 'os::Bsd::SuspendResume::SR_CONTINUE' instead of 'SR_CONTINUE'). It's a little more verbose but definitely a lot cleaner this way.

I also did the following clean-up because I touched the corresponding files anyway:

src/os_cpu/windows_x86/vm/orderAccess_windows_x86.inline.hpp

- remove "#pragma warning(disable: 4035)" because there don't seem to be any missing return statement in this file anymore

src/os/bsd/vm/os_bsd.inline.hpp
src/os/linux/vm/os_linux.inline.hpp
src/os/solaris/vm/os_solaris.inline.hpp
src/os/windows/vm/os_windows.inline.hpp

- include both "atomic.hpp" and "atomic.inline.hpp". This is unnecessary, because "XXX.inline.hpp" always includes "XXX.hpp"

Finally, I've also updated the copyright line to 2013 in every file touched by this change.

Legend: Modified file
Deleted file
New file

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/bsd/vm/os_bsd.cpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
8 lines changed: 0 ins; 0 del; 8 mod; 4542 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/bsd/vm/os_bsd.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
31 lines changed: 8 ins; 19 del; 4 mod; 261 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/bsd/vm/os_bsd.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
18 lines changed: 17 ins; 1 del; 0 mod; 288 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/linux/vm/os_linux.cpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
8 lines changed: 0 ins; 0 del; 8 mod; 5484 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/linux/vm/os_linux.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
32 lines changed: 8 ins; 20 del; 4 mod; 319 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/linux/vm/os_linux.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
18 lines changed: 17 ins; 1 del; 0 mod; 290 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/solaris/vm/os_solaris.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
1 line changed: 0 ins; 1 del; 0 mod; 263 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/windows/vm/decoder_windows.cpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 1 ins; 0 del; 1 mod; 195 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os/windows/vm/os_windows.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
1 line changed: 0 ins; 1 del; 0 mod; 109 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/bsd_x86/vm/atomic_bsd_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 219 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/bsd_x86/vm/orderAccess_bsd_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
3 lines changed: 1 ins; 0 del; 2 mod; 213 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/bsd_zero/vm/atomic_bsd_zero.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 321 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/linux_sparc/vm/atomic_linux_sparc.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 216 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/linux_x86/vm/atomic_linux_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 219 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
3 lines changed: 1 ins; 0 del; 2 mod; 213 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/linux_zero/vm/atomic_linux_zero.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 315 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/solaris_sparc/vm/atomic_solaris_sparc.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 390 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/solaris_sparc/vm/orderAccess_solaris_sparc.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 1 ins; 0 del; 1 mod; 133 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/solaris_x86/vm/atomic_solaris_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 263 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/solaris_x86/vm/orderAccess_solaris_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 0 del; 2 mod; 137 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/windows_x86/vm/atomic_windows_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 0 ins; 1 del; 1 mod; 286 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/os_cpu/windows_x86/vm/orderAccess_windows_x86.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
7 lines changed: 1 ins; 4 del; 2 mod; 213 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/memory/allocation.inline.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
2 lines changed: 1 ins; 0 del; 1 mod; 110 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/oops/symbol.cpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
24 lines changed: 23 ins; 0 del; 1 mod; 238 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/oops/symbol.hpp

rev 4205 : Fix non-PCH build on Linux, Windows and MacOS X
26 lines changed: 0 ins; 23 del; 3 mod; 231 unchg

This code review page was prepared using /sapmnt/home1/d046063/webrev.ksh (vers 23.18-hg).