RFR: 8072911: Remove includes of oop.inline.hpp from .hpp files
Bengt Rutisson
bengt.rutisson at oracle.com
Wed Feb 11 15:51:49 UTC 2015
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,
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