RFR (M/L): JDK-8035400: Move G1ParScanThreadState into its own files
bengt.rutisson at oracle.com
Wed May 7 11:18:29 UTC 2014
On 2014-04-18 15:52, Thomas Schatzl wrote:
> Hi all,
> can I have reviews for the above change? It moves G1ParScanThreadState
> into G1ParScanThreadState*pp files.
> The only changes are limited to:
> - adding a "#pragma warning( disable:4355 ) // 'this' : used in base
> member initializer list" to shut visual C up about the problem (which
> should be cleaned up at some point - I found an issue that slipped
> through because of that, JDK-8040977)
As I commented in the review of JDK-8040977 I would prefer to make the
change to not pass this as a parameter to the constructor. That would
also remove the need for disabling the warning. Maybe in that case base
this review on top of the fix for JDK-8040977 rather than the other way
> - added necessary include file references; I hope the AIX guys can
> compile that change to avoid troubles. It compiles fine with all Oracle
> supported archs.
You also moved the definition of the destructor of G1ParScanThreadState
from the hpp file to the cpp file. Makes sense, but was not strictly
needed for this change, right?
> There will be another CR for fixing up visibility and cleaning up stuff
> a little.
It is a bit hard to review moved code. But except for the comment
regarding JDK-8040977 above I think it looks good.
I think you can clean up the includes a bit more if you have time. Seems
like these includes in g1CollectedHeap.cpp are for example not needed
> perf testing indicated no changes, jprt
More information about the hotspot-gc-dev