RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy()
Mandy Chung
mandy.chung at oracle.com
Tue Oct 8 19:24:35 UTC 2019
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
More information about the core-libs-dev
mailing list