RFR: 8072911: Remove includes of oop.inline.hpp from .hpp files
coleen.phillimore at oracle.com
Thu Feb 12 03:44:26 UTC 2015
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.
> 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
> This guide line is documented on:
> 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
> Summary of the code changes:
> - Added to needed .cpp and .inline.hpp files.
> - This file isn't needed anymore and the contents have been moved to
> - 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
> - 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() ?
> - Moved the VM_GC_Operation destructor to the cpp file.
> - Moved print_on_error
> - Moved the inline keyword from the hpp file to the inline.hpp file.
> - Moved inline_write_ref_field to the inline.hpp file.
> - Moved obj_at to the inline.hpp file.
> - 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
More information about the hotspot-dev