RFR (S): 8035329: Move G1ParCopyClosure::copy_to_survivor_space into G1ParScanThreadState
Stefan Karlsson
stefan.karlsson at oracle.com
Thu Feb 20 11:55:48 UTC 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