RFR (XS): 8145674: Fix includes and forward declarations in g1Remset files
Volker Simonis
volker.simonis at gmail.com
Fri Dec 18 11:22:00 UTC 2015
Hi Thomas,
I like such kind of changes, nevertheless I have some comments:
in g1RemSet.hpp the following classes:
G1BlockOffsetSharedArray
G1SATBCardTableModRefBS
outputStream
are not used or referenced, so there's no need to forward declare them.
If files which include g1RemSet.hpp need them, than they should
forward declare the classes (and only the classes) they really require
themselves. There's no need to provide forward declarations in a
header file to fulfil requirements of users of that header file.
You added an include of "gc/g1/heapRegion.hpp" to g1RemSet.hpp but
also a forward declaration for "class HeapRegion". So either the
forward declaration of HeapRegion is enough to fulfil all the
requirements in heapRegion.hpp in which case you could remove the
include or the include is really needed (why?) in which case you can
remove the forward declaration because heapRegion.hpp defines
HeapReagion. From a first look it seems you are only using HeapRegion
pointers in g1RemSet.hpp so you should probably drop the inclusion of
heapRegion.hpp.
The other extra includes are fine.
The inclusion of "gc/g1/heapRegion.inline.hpp" into g1RemSet.cpp is
also fine because g1RemSet.cpp uses the inline function
HeapRegion::in_collection_set() which is defined in
"gc/g1/heapRegion.inline.hpp"
Regards,
Volker
On Fri, Dec 18, 2015 at 11:48 AM, Thomas Schatzl
<thomas.schatzl at oracle.com> wrote:
> Hi all,
>
> can I have reviews for this change that adds necessary includes and
> forward declarations in g1RemSet.?pp files? While it compiles right now
> as is, if something changes in this area, compilation tends to break in
> interesting ways.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8145674
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8145674/webrev/
> Testing:
> jprt
>
> Thanks,
> Thomas
>
More information about the hotspot-gc-dev
mailing list