RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy()
OGATAK at jp.ibm.com
Wed Aug 21 11:02:40 UTC 2019
Thank you for reviewing the webrev. I updated it to add a space after
"if" and also put four spaces for indentation (it was three).
Thank you so much for checking the history of fieldAccessor. I was
surprised that fieldAccessor was made non-volatile in JDK5, but
methodAccessor was left as volatile for 15 years after that...
I agree we need benchmark data. My simple micro benchmark that repeats
invoking Class.getMethods() improved performance by 70% when it made
non-volatile (as shown in the following webrev). I'll try to run larger
benchmarks, such as SPECjbb2015, to see real impact.
Mandy Chung <mandy.chung at oracle.com> wrote on 2019/08/21 01:21:42:
> From: Mandy Chung <mandy.chung at oracle.com>
> To: Kazunori Ogata <OGATAK at jp.ibm.com>
> Cc: core-libs-dev at openjdk.java.net
> Date: 2019/08/21 01:21
> Subject: [EXTERNAL] Re: RFR: JDK-8229871: Imporve performance of
> Method.copy() and leafCopy()
> Hi Ogata,
> The patch looks okay. Nit: please add a space between if and (.
> About volatile methodAccessor field, I checked the history. Both
> fieldAccessor and methodAccessor were started as volatile and the
> fieldAccessor declaration was updated due to JDK-5044412. As you
> observe, I think the methodAccessor field could be made non-volatile.
> OTOH that might impact when it's inflated to spin bytecode for this
> method invocation. I don't know how importance to keep its volatile vs
> non-volatile in practice without doing benchmarking/real application
> On 8/19/19 2:51 AM, Kazunori Ogata wrote:
> > Hi,
> > May I have review for "JDK-8229871: Imporve performance of
> > and leafCopy()"?
> > Method.copy() and leafCopy() creates a copy of a Method object with
> > sharing MethodAccessor object. Since the methodAccessor field is a
> > volatile variable, copying this field needs memory fence to ensure the
> > field is visible to all threads on the weak memory platforms such as
> > and ARM.
> > When the methodAccessor of the root object is null (i.e., not
> > yet), we do not need to copy the null value because this field of the
> > copied object has been initialized to null in the constructor. We can
> > reduce overhead of the memory fence only when the root's
> > non-null. This change improved performance by 5.8% using a micro
> > that repeatedly invokes Class.getMethods().
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8229871
> > Webrev: http://cr.openjdk.java.net/~ogatak/8229871/webrev.00/
> > By the way, why Method.methodAccessor is volatile, while
> > Field.fieldAccessor and Field.overrideFieldAccessor are not volatile?
> > know the use of volatile reduces probability of creating duplicated
> > accessor, but the chance still exists. I couldn't find the difference
> > between Method and Field classes to make Method.methodAccessor
> > If we can make it non-volatile, it is more preferable than a quick
> > above.
> > Regards,
> > Ogata
More information about the core-libs-dev