RFR: 8072911: Remove includes of oop.inline.hpp from .hpp files

[David Holmes]

Hi Jesper,

On 12/02/2015 3:56 AM, Volker Simonis wrote:
> Hi Stefan,
>
> thanks for doing this change and thanks for changing the ppc-files as well.
>
> Unfortunately there still seems to be an error if building on
> Linux/ppc64 without precompiled headers.

I was going to ask if this had been tested with PCH disabled. (Not that 
we'd detect this particular issue.)

Otherwise changes seem okay to me - the proof si in the builoding :)

One minor comment typo in oop.hpp and oop.cpp:

  // type test operations that doesn't require inclusion of oop.inline.hpp.

s/doesn't/don't/

Thanks,
David

> Can you please wait a little until I'll send you the additional
> changes which are required before pushing this one.
>
> Thanks,
> Volker
>
> PS: I've just checked that AIX/ppc64 builds without a problem.
>
> On Wed, Feb 11, 2015 at 5:57 PM, Jesper Wilhelmsson
> <jesper.wilhelmsson at oracle.com> wrote:
>> Looks good!
>> /Jesper
>>
>> Stefan Karlsson skrev den 11/2/15 16:55:
>>
>>> Hi Bengt,
>>>
>>> On 2015-02-11 16:51, Bengt Rutisson wrote:
>>>>
>>>>
>>>> Hi Stefan,
>>>>
>>>> Pretty difficult to review this type of change, but I think it looks
>>>> good. I
>>>> verified that no hpp files inlcude oop.inline.hpp after your change. Nice
>>>> cleanup!
>>>>
>>>> One minor thing (not your code):
>>>>
>>>> In src/share/vm/oops/oop.hpp there is this comment:
>>>>
>>>>    // type test operations (inlined in oop.inline.h)
>>>>
>>>> I think the comment should say oop.inline.hpp not just .h. Do you mind
>>>> fixing
>>>> this as part of this change too?
>>>
>>>
>>> Thanks for reviewing. I'll fix the comment.
>>>
>>> Thanks,
>>> StefanK
>>>
>>>>
>>>> Thanks,
>>>> Bengt
>>>>
>>>>
>>>>
>>>> On 11/02/15 13:38, Stefan Karlsson wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Please, review this patch to fix our includes of oop.inline.hpp.
>>>>>
>>>>> http://cr.openjdk.java.net/~stefank/8072911/webrev.01
>>>>> https://bugs.openjdk.java.net/browse/JDK-8072911
>>>>>
>>>>>
>>>>>  From the enhancement description:
>>>>> ---
>>>>> Our inline.hpp files should preferably only be included from .cpp files
>>>>> or
>>>>> other .inline.hpp files. When they are included in .hpp files, the
>>>>> include
>>>>> dependency of the implementation spreads to unrelated parts of the code
>>>>> base,
>>>>> and it's easy to get circular dependencies.
>>>>>
>>>>> This guide line is documented on:
>>>>> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#Files
>>>>>
>>>>> oop.inline.hpp is one of the more problematic files, and I propose a
>>>>> patch to
>>>>> get rid of all inclusions of oop.inline.hpp from other .hpp files.
>>>>> ---
>>>>>
>>>>>
>>>>> Summary of the code changes:
>>>>>
>>>>> oop.inline.hpp
>>>>>   - Added to needed .cpp and .inline.hpp files.
>>>>>
>>>>> oop.inline2.hpp
>>>>>   - This file isn't needed anymore and the contents have been moved to
>>>>> oop.inline.hpp.
>>>>>
>>>>> oop.hpp
>>>>>   - Inlined klass_gap_offset_in_bytes in the hpp file, since it's
>>>>> currently
>>>>> used from a few hpp files.
>>>>>
>>>>>   - Create non-inlined versions of is_instance checks, to be used in
>>>>> asserts
>>>>> in handles.hpp.
>>>>>
>>>>>   - Added has_klass_gap to get rid of dependency against globals.hpp
>>>>>
>>>>> javaClasses.hpp
>>>>>   - There were a couple of usages of klass.hpp that were moved out to
>>>>> javaClasses.cpp and a few that were moved to javaClasses.inline.hpp when
>>>>> it
>>>>> wasn't obvious if we needed to inline the functions or not.
>>>>>
>>>>> vmGCOperations.hpp
>>>>>   - Moved the VM_GC_Operation destructor to the cpp file.
>>>>>
>>>>> collectedHeap.hpp
>>>>>   - Moved print_on_error
>>>>>
>>>>> barrierSet.hpp
>>>>>   - Moved the inline keyword from the hpp file to the inline.hpp file.
>>>>>
>>>>> cardTableModRefBS.hpp
>>>>>   - Moved inline_write_ref_field to the inline.hpp file.
>>>>>
>>>>> objArrayOop.hpp
>>>>>   - Moved obj_at to the inline.hpp file.
>>>>>
>>>>> graphKit.hpp
>>>>>   - Moved byte_map_base_node to .cpp file, where it's used.
>>>>>
>>>>> <other added includes>
>>>>>   - Added missing includes that were removed when oop.inline.hpp was
>>>>> removed
>>>>> from the hpp files.
>>>>>
>>>>>
>>>>> The patch has been tested with JPRT.
>>>>>
>>>>> Thanks,
>>>>> StefanK
>>>>>
>>>>
>>>
>>