6348631 - request for review (updated)

[Ivan Krylov]

David,
On 02.12.2010 1:15, David Holmes wrote:
> Hmmm
>
> coleen phillimore said the following on 12/01/10 04:28:
>> This looks good to me.  I have some concerns with the inlined socket functions in:
>>
>> http://cr.openjdk.java.net/~ikrylov/6348631.v3/src/os/linux/vm/os_linux.inline.hpp.cdiff.html
>>
>> as it causes the .hpp file to import linux system specific hpp files which might cause namespace issues down the line.  I'd rather these sorts of 
>> includes be in a .cpp file.  I don't know if they were inlined for performance reasons though, but it seems unlikely to have any benefit.
>
> When I was taught C programming I was taught that each file should be self-contained and include all headers that it needed. That way if I include a 
> header to get the definitions for the "interface" to some library, I don't have to know what all of its dependencies are - I just include one header 
> and call the library functions. I don't have to discover all its dependencies by trial and error (nor does the provider of the library have to 
> document them all).
>
> Arguably for our xxx.inline.hpp files it is a different situation because they should only be included by our own xxx.cpp file. But as a general 
> rule I would not advocate removing all system includes from all hotspot headers.

These files include os_linux.inline.hpp explicitly now.

./share/vm/classfile/classLoader.cpp
./share/vm/runtime/synchronizer.cpp
./share/vm/runtime/handles.cpp
./share/vm/runtime/thread.cpp
./share/vm/runtime/mutexLocker.hpp
./share/vm/runtime/timer.cpp
./share/vm/runtime/arguments.cpp
./share/vm/runtime/virtualspace.cpp
./share/vm/runtime/task.cpp
./share/vm/runtime/threadLocalStorage.cpp
./share/vm/runtime/atomic.cpp
./share/vm/runtime/os.cpp
./share/vm/runtime/objectMonitor.cpp
./share/vm/memory/space.hpp
./share/vm/memory/gcLocker.hpp
./share/vm/memory/allocation.cpp
./share/vm/prims/nativeLookup.cpp
./share/vm/prims/jni.cpp
./share/vm/compiler/disassembler.hpp
./share/vm/code/stubs.hpp
./share/vm/utilities/bitMap.cpp
./share/vm/utilities/accessFlags.cpp
./share/vm/utilities/histogram.hpp
./share/vm/utilities/ostream.cpp
./share/vm/utilities/debug.cpp
./share/vm/gc_implementation/parallelScavenge/parMarkBitMap.cpp
./share/vm/gc_implementation/parallelScavenge/psVirtualspace.cpp
./share/vm/gc_implementation/concurrentMarkSweep/cmsAdaptiveSizePolicy.cpp
./os/linux/vm/perfMemory_linux.cpp
./os/linux/vm/mutex_linux.inline.hpp
./cpu/sparc/vm/vm_version_sparc.cpp
./cpu/zero/vm/vm_version_zero.cpp
./cpu/x86/vm/vm_version_x86.cpp

Even worse: when gcc generates a precompiled header it wants to see definitions of those system structures and functions.That is before any cpp file 
is being parsed.

Thanks,
Ivan