Request for review (S): 7130974 G1: Remove G1ParCopyHelper

[Tony Printezis]

Bengt,

Hi, the change looks good. The only concern I have is that by moving 
copy_to_survivor_space() into G1ParCopyClosure, it will be compiled 
several times (and it's not a small method), once for every 
instantiation of the template (of which we have quite a few IIRC). On 
the other hand, this way we'll also get more more accurate profiles. 
E.g., we'll get a better idea where copy_to_survivor_space() is called 
from, i.e., when scanning roots? RSets? etc. This could be very 
valuable.. Additionally, I've looked at collect/analyze G1 profiles in 
the past and I've seen cases where copy_to_survivor_space() was being 
inlined in the caller, so maybe the code is already replicated. Anyway, 
I'm thinking aloud. I think it's OK but I thought that I should bring 
this up.

Cosmetic suggestion: maybe you want to split some of the lines that got 
quite long with the introduction of the template parameters?

4391 template<bool do_gen_barrier, G1Barrier barrier, bool do_mark_object>
4392 oop G1ParCopyClosure<do_gen_barrier, barrier, do_mark_object>::copy_to_survivor_space(oop old) {


Tony

On 03/13/2012 07:49 AM, Bengt Rutisson wrote:
>
> Hi all,
>
> Could I have a couple of reviews of this small code clean up?
>
> http://cr.openjdk.java.net/~brutisso/7130974/webrev.00/
>
> Using templates it is possible to get rid of the extra inheritance 
> level introduced by G1ParCopyHelper. This also removes a dependency 
> between G1ParCopyHelper and G1ParCopyClosure regarding how the 
> _scanner field is initialized.
>
> Bug report:
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7130974
>
> Thanks,
> Bengt