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

[Lindenmaier, Goetz]

Sure!  I updated the webrev accordingly.

Best regards,
  Goetz.

From: Stefan Karlsson [mailto:stefan.karlsson at oracle.com]
Sent: Dienstag, 1. April 2014 12:07
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,
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<mailto:hotspot-dev at openjdk.java.net>; hotspot-gc-dev at openjdk.java.net<mailto: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/5e8c111c/attachment.htm>