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

Mandy Chung mandy.chung at oracle.com
Tue Aug 20 16:21:42 UTC 2019


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 
testing.

Mandy

On 8/19/19 2:51 AM, Kazunori Ogata wrote:
> Hi,
>
> May I have review for "JDK-8229871: Imporve performance of Method.copy()
> 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 POWER
> and ARM.
>
> When the methodAccessor of the root object is null (i.e., not initialized
> 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 methodAccessor is
> non-null. This change improved performance by 5.8% using a micro benchmark
> 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?  I
> know the use of volatile reduces probability of creating duplicated method
> accessor, but the chance still exists.  I couldn't find the difference
> between Method and Field classes to make Method.methodAccessor volatile.
> If we can make it non-volatile, it is more preferable than a quick hack
> above.
>
>
> Regards,
> Ogata
>



More information about the core-libs-dev mailing list