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

[Stefan Karlsson]

Hi Goetz,

On 2014-04-01 11:53, Lindenmaier, Goetz wrote:
>
> Hi Stefan,
>
> thanks for pointing me to that.
>
> I think I did that to break the chain at some point.
>
> But I only need to move two more functions so that all is
>
> in the .inline file:
>
> http://cr.openjdk.java.net/~goetz/webrevs/8038498-incl/webrev.01/ 
> <http://cr.openjdk.java.net/%7Egoetz/webrevs/8038498-incl/webrev.01/>
>

I see that you had to move the two G1CollectedHeap::is_obj_dead_cond 
functions out of g1CollectedHeap.hpp. I don't think they have to be 
inline functions, since they are only used by the verification code. 
Would you mind moving them to the g1CollectedHeap.cpp instead? Other 
than that, I think the patch looks good.

thanks!
StefanK

> Best regards,
>
>   Goetz.
>
> *From:*Stefan Karlsson [mailto:stefan.karlsson at oracle.com]
> *Sent:* Dienstag, 1. April 2014 10:34
> *To:* Lindenmaier, Goetz; hotspot-dev at openjdk.java.net; 
> hotspot-gc-dev at openjdk.java.net
> *Subject:* Re: RFR(M): 8038498: Fix includes and C inlining after 8035330
>
> Hi Goetz,
>
> Thanks for fixing this!
>
> One comment below:
>
> On 2014-03-27 17:45, Lindenmaier, Goetz wrote:
>
>     Hi,
>
>     Please review and test this change.  I please need a sponsor:
>
>     http://cr.openjdk.java.net/~goetz/webrevs/8038498-incl/
>     <http://cr.openjdk.java.net/%7Egoetz/webrevs/8038498-incl/>
>
>
> http://cr.openjdk.java.net/~goetz/webrevs/8038498-incl/webrev.00/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.udiff.html 
> <http://cr.openjdk.java.net/%7Egoetz/webrevs/8038498-incl/webrev.00/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.udiff.html>
>
> I'm not sure what the performance implication is of moving these 
> functions to the .cpp file. Is there a reason why we wouldn't want to 
> put them in the g1CollectedHeap.inline.hpp file?
>
> thanks,
> StefanK
>
>
> 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20140401/e1e1e3b8/attachment.htm>