RFR (M): 8035330: Remove G1ParScanPartialArrayClosure and G1ParScanHeapEvacClosure

[Stefan Karlsson]

On 2014-02-20 11:00, Thomas Schatzl wrote:
> Hi Stefan,
>
> On Thu, 2014-02-20 at 10:21 +0100, Stefan Karlsson wrote:
>> Hi Thomas,
>>
>> On 2014-02-19 16:13, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     can I have reviews for the following change that changes the
>>> G1ParScanPartialArrayClosure and G1ParScanHeapEvacClosure closures into
>>> methods of G1ParScanThreadState?
>>>
>>> G1ParScanPartialArrayClosure and G1ParScanHeapEvacClosure are two
>>> closures which do_oop_nv method is always called directly and from a
>>> single location, i.e. G1ParScanThreadState.
>>>
>>> This change removes the boiler plate code of these "closures" to avoid
>>> more confusion (G1ParScanHeapEvacClosure was a candidate for oop closure
>>> specialization although it was never called on an oop_iterate() method),
>>> makes the do_oop_nv() methods into two methods of G1ParScanThreadState,
>>> in addition to moving related code also into the vicinity.
>>>
>>> Another minor change is that do_oop_evac() (the replacement for
>>> G1ParScanHeapEvacClosure::do_oop_nc()) is stripped of the not-required
>>> NULL check.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8035330
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8035330/webrev/
>> You move a lot of code into g1CollectedHeap.hpp. Could you change the
>> code so that it ends up in the .cpp (or .inline.hpp) file instead?
>    I was thinking about, and Jon already suggested to move
> G1ParScanThreadState into separate file(s) in a separate changeset, is
> this fine with you?

Sure. That's fine with me. As long as you don't put all the 
implementation in the .hpp file.

>
> Another follow-up on that would be fixing the visibility of the members
> and methods of G1ParScanThreadState. I saw a lot of things were, and are
> now unnecessarily public.

That would be great.

>
> I hope you do not mind the many cleanup CRs then.

No, I much prefer to review it this way. It's also much easier to do 
code archeology if something goes wrong.

thanks a lot,
StefanK

>
> Thanks,
>    Thomas
>
>