RFR (M): 8035330: Remove G1ParScanPartialArrayClosure and G1ParScanHeapEvacClosure
thomas.schatzl at oracle.com
Thu Feb 20 02:00:21 PST 2014
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?
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.
I hope you do not mind the many cleanup CRs then.
More information about the hotspot-gc-dev