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

[Coleen Phillimore]

I filed an RFE for this.   Thank you for the specifics.   We have a 
label/keyword "starter" that we've been using to give new contributors 
ideas for how they can get started on the JVM.   I'll give this that one 
(even though it's a lot of code changes!)

IssueJDK-8003990 - Clean up inline #includes 
<https://jbs.oracle.com/bugs/browse/JDK-8003990>has been successfully 
created.

If someone wants to start on this from the community, let me know, so 
nobody starts on it from Oracle at the same time.
Thanks,
Coleen

On 11/26/2012 10:59 AM, 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).
>
> Are there any plans to further clean this up (i.e. remove inline.hpp
> includes from .hpp files) by either:
>
>   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
>>>>
>>>>