RFR(M): 8038498: Fix includes and C inlining after 8035330

[David Holmes]

Hi Goetz,

Not a review ...

On 31/03/2014 10:42 PM, Lindenmaier, Goetz wrote:
> Hi,
>
> could I please get reviews for this change?  I please also need a sponsor.
> I would like to get this into 8u20, as 8036330 was downported, too.
>
> I'd like to make the point of this change clear once more, independent
> of which compiler does what.
>
> I think we agree that basically
>    - a file should include all headers that define symbols or include
>      method bodies used in that file

Personally I don't see why a cpp file should have to be aware that a 
member function it uses just happens to have been defined in an 
inline.hpp file by its author. Seems to break encapsulation to me. :(

>    - a .hpp header should not include a .inline.hpp header

I see 277 occurrences of this in the existing OpenJDK hotspot code, so I 
don't think we can take this as a general rule either.

Note I'm not saying this fix isn't right or needed, just that the 
"rules" you are trying to embody don't seem to generally apply.

David
-----

> This is violated
>
> 1.)    by defining
>   template <class T> void immediate_rs_update(HeapRegion* from, T* p, int tid) {
>     if (!from->is_survivor()) {
>        _g1_rem->par_write_ref(from, p, tid);
>      }
> }
>   in g1CollectedHeap.hpp, as par_write_ref in declared 'inline' and only defined in
>      g1RemSet.inline.hpp - which is not included and should not be included
>      in g1CollectedHeap.hpp.
> 2. by including g1CollectedHeap.inline.hpp in sparsePRT.hpp
>
> This change fixes these two points by moving functions into .inline.hpp files
> and fixing the include.
>
> Best regards,
>    Goetz.
>
>
>
>
> From: Lindenmaier, Goetz
> Sent: Donnerstag, 27. März 2014 17:46
> To: hotspot-dev at openjdk.java.net; hotspot-gc-dev at openjdk.java.net
> Subject: RFR(M): 8038498: Fix includes and C inlining after 8035330
>
> Hi,
>
> Please review and test this change.  I please need a sponsor:
> http://cr.openjdk.java.net/~goetz/webrevs/8038498-incl/
>
> Change 8035330: Remove G1ParScanPartialArrayClosure and G1ParScanHeapEvacClosure
> broke the dbg build on AIX. That's because do_oop_partial_array() is added in
> a header, but requires inline function par_write_ref() through several inlined
> calls. In some cpp files, like arguments.cpp, par_write_ref() is not defined
> as the corresponding inline header is not included.  The aix debug VM does not start.
>
> This can be solved by including g1RemSet.inline.hpp in g1CollectedHeap.inline.hpp.
>
> Unfortunately this now causes a cyclic dependency that breaks the linux build.
> A inline.hpp file is included ahead of a .hpp file, so that the code in the inline.hpp file
> can not see the class declaration.
> This is caused because g1CollectedHeap.inline.hpp is included in sparsePRT.hpp.
> But .inline.hpp files never should be included in .hpp files.
>
> To resolve this, I changed this inlcude to g1CollectedHeap.hpp.  As consequence,
> I had to move a row of functions to existing .inline.hpp files.
>
> I did debug, fastdebug and product builds on linux_ppc64, aix_ppc64,
> sun_64, bsd_x86_64 and linux_x86_64 and tested that the VM starts up.
>
> Best regards,
>    Goetz
>