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

Volker Simonis volker.simonis at gmail.com
Wed Feb 11 17:56:10 UTC 2015


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.
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
>>>>
>>>
>>
>


More information about the hotspot-dev mailing list