RFR (XS): 8218192: Remove copy constructor for MemRegion

[Man Cao]

Thank you both for the reviews and suggestions.
I will add such comment in this patch.
It is really helpful to know these little details.

For those other places that pass MemRegions by const-ref,
do you think we should change them to pass-by-value if we
validate that the compiler generates the same code as pass-by-ref?

I still think it might be better to just pass by const-ref for most
places, as it will prevent this issue from happening even if the
compiler failed to apply the optimization for pass-by-value.

-Man


On Tue, Feb 5, 2019 at 10:42 AM Kim Barrett <kim.barrett at oracle.com> wrote:

> > On Feb 5, 2019, at 11:25 AM, Man Cao <manc at google.com> wrote:
> >
> > Hi all,
> >
> > Could I have reviews for this small patch?
> > Webrev: https://cr.openjdk.java.net/~manc/8218192/webrev.00/
> > RFE: https://bugs.openjdk.java.net/browse/JDK-8218192
> >
> > This will improve G1 write barrier performance for jython in DaCapo,
> when building HotSpot with LLVM. More Background and performance numbers
> are included in the JBS.
> >
> > Thanks,
> > Man
>
> Looks good.
>
> The only thing I might suggest is adding a comment that the destructor
> and copy constructor must be trivial, to support optimization of pass
> by value.  I don't need a new webrev if you add such a comment.
>
> The explicit copy constructor is clearly interfering with the expected
> optimization on Linux-x64, and likely other platforms where I didn't
> examine the generated assembly.  The SysV ABI describes passing such
> an object in a pair of registers for the members, but only when the
> destructor and copy constructor are trivial.  (An older version of the
> ABI required the class to be POD, which is unnecessarily restrictive;
> gcc through 4.9 (I think) used that.)
>
> There are a handful of places that are passing MemRegions by const-ref
> rather than by value that probably ought to be looked at, but that can
> be done separately.
>
> I did some performance testing of this change as well.  Seemed pretty
> much in the noise.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190206/0e442297/attachment.htm>