RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy()

[Mandy Chung]

On 10/8/19 6:31 AM, Claes Redestad wrote:
> Hi,
>
> webrev.02 looks good to me.
>

webrev.02 looks fine to me as well.

> I think the performance results makes sense since avoiding a volatile
> store (and the potentially expensive store barriers this involves)
> should be the main benefit. Adding a branch to avoid storing null would
> help partially, but not hot Methods.
>
> Pre-existing issue, but it's somewhat weird that we have two assignments
> outside the package-private constructor: adding root and methodAccessor
> to the constructor would make more immediate sense to me, since we do
> the same thing at the only two callsites:
>
>         Method res = new Method(clazz, name, parameterTypes, returnType,
>                 exceptionTypes, modifiers, slot, signature,
>                 annotations, parameterAnnotations, annotationDefault);
>         res.root = root;
>         res.methodAccessor = methodAccessor;
>
> ->
>
>         Method res = new Method(clazz, name, parameterTypes, returnType,
>                 exceptionTypes, modifiers, slot, signature,
>                 annotations, parameterAnnotations, annotationDefault,
>                 root, methodAccessor);
>
> This package-private constructor could also be made private. My guess is
> there was some time when this was used from outside Method and changing
> it was deemed unsavory..

The parameters of the Method constructor are the content from a 
ClassFile to represent a method whereas root and methodAccessor fields 
are not part of it. The current implementation allocates a Method 
instance and sets the fields individually instead of calling the 
constructor probably due to bootstrapping.

I suggest to keep it as it is.  OTOH making the constructor private is fine.

Mandy