RFR (S): 8035329: Move G1ParCopyClosure::copy_to_survivor_space into G1ParScanThreadState

Thomas Schatzl thomas.schatzl at oracle.com
Thu Feb 20 02:22:57 PST 2014


Hi Stefan,

  thanks for looking at this...

On Thu, 2014-02-20 at 10:11 +0100, Stefan Karlsson wrote:
> Hi Thomas,
> 
> On 2014-02-19 16:12, Thomas Schatzl wrote:
> > Hi all,
> >
> >    can I have reviews for the following change that moves
> > G1ParCopyClosure::copy_to_survivor_space() into G1ParScanThreadState.
> >
> > The previous location of G1ParCopyClosure::copy_to_survivor_space() was
> > bad because although it did not depend on any template parameters, it
> > was there, resulting in additional duplicate code.
> >
> > As for the new destination, G1ParScanThreadState seemed to be the best
> > candidate because the method references it a lot (calling a few methods
> > from it).
> >
> > Also, the next change for JDK-8035330 also depends on _scanner being
> > available in G1ParScanThreadState.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8035329
> >
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8035329/webrev/
> 
> This patch doesn't look complete. It doesn't remove _scanner and 
> copy_to_survivor_space from G1ParCopyClosure.

Just the declarations. Sorry, overlooked that when splitting the
changes. Thanks!

New webrev at http://cr.openjdk.java.net/~tschatzl/8035329/webrev.1/

Diff:

--- a/src/share/vm/gc_implementation/g1/g1OopClosures.hpp	Wed Feb 19
15:35:27 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/g1OopClosures.hpp	Thu Feb 20
10:43:27 2014 +0100
@@ -151,22 +151,16 @@
 
 template <G1Barrier barrier, bool do_mark_object>
 class G1ParCopyClosure : public G1ParCopyHelper {
-  G1ParScanClosure _scanner;
+private:
   template <class T> void do_oop_work(T* p);
 
-protected:
-  oop copy_to_survivor_space(oop obj);
-
 public:
   G1ParCopyClosure(G1CollectedHeap* g1, G1ParScanThreadState*
par_scan_state,
                    ReferenceProcessor* rp) :
-      _scanner(g1, par_scan_state, rp),
       G1ParCopyHelper(g1, par_scan_state) {
     assert(_ref_processor == NULL, "sanity");
   }
 
-  G1ParScanClosure* scanner() { return &_scanner; }
-
   template <class T> void do_oop_nv(T* p) { do_oop_work(p); }
   virtual void do_oop(oop* p)       { do_oop_nv(p); }
   virtual void do_oop(narrowOop* p) { do_oop_nv(p); }


Thanks,
Thomas




More information about the hotspot-gc-dev mailing list