RFR (M/L): JDK-8035400: Move G1ParScanThreadState into its own files

[Bengt Rutisson]

Hi Thomas,

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 
around?

>   - 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.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8035400
>
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8035400/webrev/

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 
anymore:

#include "oops/oop.inline.hpp"
#include "oops/oop.pcgc.inline.hpp"

Thanks,
Bengt

>
> Testing:
> perf testing indicated no changes, jprt
>
> Thanks,
>    Thomas
>
>
>