RFR (XL): 8208669: GC changes to allow enabling -Wreorder
Kim Barrett
kim.barrett at oracle.com
Fri Aug 3 20:41:36 UTC 2018
> On Aug 2, 2018, at 5:07 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>
> Hi all,
>
> there have been several suggestions to try to fix the Hotspot code to
> allow us to enable -Wreorder in the Hotspot sources.
> This should make problems due to use-before-initialization much more
> obvious.
>
> This change fixes initializer lists for the GC directories.
>
> There should be no change in code behavior, except for that there is
> iirc one minor change that fixes the memory type of one initialization
> from mtInternal to mtGC.
>
> A later follow-up will enable -Wreorder in the makefiles.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8208669
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8208669/webrev/
> Testing:
> hs-tier1-4,jdk-tier1-3
>
> Thanks,
> Thomas
Looks good.
A few comments and minor things for which I don't need a new webrev.
There were some additions of explicit initialization in the
initializer list, that didn't seem associated with -Wreorder. I'm
okay with them.
------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1BlockOffsetTable.cpp
80 #ifdef ASSERT
81 _object_can_span(false),
82 #endif
FYI, this can also be written as
DEBUG_ONLY(_object_can_span(false) COMMA)
------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
1423 _expansion_regions(0),
This is an additional explicit initialization. However, this member
is never used.
------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
1455 _evacuation_failed(false),
This is adding an explicit initialization in the initializer list,
where there wasn't even an initialization assignment in the
constructor body. It was okay before because it is assigned in
pre_evacuate_collection_set. I'm okay with this, just confirming
intent.
------------------------------------------------------------------------------
src/hotspot/share/gc/cms/parNewGeneration.cpp
97 _hash_seed(0),
src/hotspot/share/gc/g1/g1RegionMarkStatsCache.cpp
36 _num_cache_entries_mask(0) {
src/hotspot/share/gc/shared/workerDataArray.inline.hpp
35 _length(0),
Rather than initializing to a dummy value here and keeping the later
in-body assignment, move the in-body value to the initializer.
------------------------------------------------------------------------------
src/hotspot/share/gc/shared/workgroup.cpp
190 MutexGangTaskDispatcher()
I don't really like how this one ended up being re-formated,
especially having the (empty) constructor body way far over to the
right. Also, aligning the colon with the constructor name seems
pretty unusual.
------------------------------------------------------------------------------
More information about the hotspot-dev
mailing list