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

[Coleen Phillimore]

Hi Stefan,
I think these cleanups are great.

On 2/11/15, 7:38 AM, 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.

Hooray!!

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

Why not put klass_gap_offset_in_bytes in the .cpp file?  It's not going 
to be inlined completely anyway because the new version is calling 
has_klass_gap() which is in the .cpp file.  I don't think it's usage is 
performance critical.

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

I'm confused about this.  Why is there an 
java_lang_String::is_instance_inlined() and an is_instance() ?

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

JPRT compiles without precompiled headers on at least one of the solaris 
platforms, I think.  You could compile on linux without precompiled 
headers just for verification locally.

This change looks awesome.   Thank you for doing this cleanup!   Can you 
update the copyright headers when you check it in?  That should suppress 
this "please update the copyright headers" comment on all these files 
going forward.

Thanks!
Coleen
>
> Thanks,
> StefanK
>