Review request: 8003935: Simplify the needed includes for using Thread::current()

[Stefan Karlsson]

On 11/26/2012 04:59 PM, Volker Simonis wrote:
> On Mon, Nov 26, 2012 at 8:05 AM, Stefan Karlsson
> <stefan.karlsson at oracle.com> wrote:
>> On 2012-11-23 13:22, David Holmes wrote:
>>> Hi Stefan,
>>>
>>> I never did like these.
>>>
>>> I have to wonder though, why isn't it "runtime/thread.hpp" that all the
>>> client code #includes?
>> Typically, you place inline functions in a inline.hpp file instead of the
>> hpp. This helps reducing the number of unnecessary includes from spreading
>> through out the code base. Only the users of the inline functions will
>> include the inline.hpp file, and bring in the needed includes to implement
>> the function.
>>
>> Preferably, you should only include inline.hpp files from other cpp files or
>> other inline.hpp files.
>>
> This is indeed the glory theory. However even in your change, there
> are still some .hpp files which include .inline.hpp files (e.g
> abstractInterpreter.hpp includes thread.inline.hpp).

The reason for this RFE was to remove the platform dependent includes, 
not to fix the includes of .inline.hpp files from .hpp files.

However, I actually tried to remove all included of thread.inline.hpp 
from .hpp files, but I didn't proceed when I realized I had to change 
the includes for the ResourceMark. It would have been a too big change 
for this small RFE.

>
> Are there any plans to further clean this up (i.e. remove inline.hpp
> includes from .hpp files) by either:

Coleen created JDK-8003990 for that.

thanks,
StefanK

>
>   1a : move the methods from XXX.hpp which require implementations of
> inline functions from AAA.inline.hpp to XXX.cpp if they are not
> performance relevant (and include AAA.inline.hpp only in XXX.cpp).
>
>   1b : moving the methods from XXX.hpp which require implementations of
> inline functions from AAA.inline.hpp to a newly created XXX.inline.hpp
> if they are performance relevant (and include AAA.inline.hpp only in
> XXX.inline.cpp).
>
>   1c : keep the implementation of methods in XXX.hpp which don't
> require definitions of inline methods from ANY ".inline.hpp" (in this
> case it may be still necessary to include .hpp files which have been
> provided previously indirectly by the various ".inline.hpp" files)
>
> In general I would be in favour of such a change, but a quick search
> (find hotspot/src/ -name "*.hpp" | grep -v "\.inline\.hpp" | xargs
> grep ".inline.hpp" | awk -F ":" '{print $1;}' | sort -u | wc) reveals
> that there are currently 74 ".hpp" files which include other
> ".inline.hpp" files. So this would be a real huge change.
>
>>> And then why isn't it thread.hpp that includes the platform specific
>>> header?
>>
>> In this specific case, there's an inline function in
>> thread_solaris.inline.hpp that Thread::current() needs.
>>
>> StefanK
>>
>>> David
>>>
>>> On 23/11/2012 9:54 PM, Stefan Karlsson wrote:
>>>> http://cr.openjdk.java.net/~stefank/8003935/webrev
>>>>
>>>> Today, whenever we use Thread::current() we have to all these lines:
>>>>
>>>> #include "runtime/thread.hpp"
>>>> #ifdef TARGET_OS_FAMILY_linux
>>>> # include "thread_linux.inline.hpp"
>>>> #endif
>>>> #ifdef TARGET_OS_FAMILY_solaris
>>>> # include "thread_solaris.inline.hpp"
>>>> #endif
>>>> #ifdef TARGET_OS_FAMILY_windows
>>>> # include "thread_windows.inline.hpp"
>>>> #endif
>>>> #ifdef TARGET_OS_FAMILY_bsd
>>>> # include "thread_bsd.inline.hpp"
>>>> #endif
>>>>
>>>> This patch hides this dispatching in a new file named thread.inline.hpp.
>>>> Now we only have to include thread.inline.hpp.
>>>>
>>>> Some background to these includes: This type of dispatching was
>>>> introduced into the source files when we removed the includeDB files. We
>>>> discussed whether we should use "dispatch files" or not for this kind
>>>> platform dependent includes. The decision was to not create dispatch
>>>> files for all types of platform at that time, but instead manually
>>>> create them when we thought it was warranted.
>>>>
>>>> thanks,
>>>> StefanK
>>>>
>>>>