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

Stefan Karlsson stefan.karlsson at oracle.com
Thu Feb 20 03:55:48 PST 2014


On 2014-02-20 11:22, Thomas Schatzl wrote:
> 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); }

Looks good.

thanks,
StefanK

>
>
> Thanks,
> Thomas
>
>



More information about the hotspot-gc-dev mailing list