You might consider storing the cached item in checkAccess() in a
ThreadLocal instead of a static. Not only would this eliminate the
volatile accesses, but it reduces the chance of a "miss" in this
one-element cache, since it will then be impervious to interleavings
where other threads might use reflection. (The purpose of the cache is
to speed up the case when some code is grinding through all the members
of a Class; it is unlikely that there would be many hits in this cache
across threads anyway.) Obviously this is a tradeoff of memory
utilization for reduced synchronization traffic + higher cache hit rate
+ more predictable reflection performance under concurrent load.
You can eliminate the second read-volatile by having
acquireMethodAccessor return the MA instead of re-reading the volatile.
On 3/17/2011 7:04 PM, Mike Duigou wrote:
Sorry folks--the webrev url:
http://cr.openjdk.java.net/~mduigou/6565585/0/webrev/
Mike
On Mar 17 2011, at 15:07 , Mike Duigou wrote:
Method.invoke(), Contrstuctor.newInstance() and Field.getFieldAccessor() all
have a needless critical section, causing large slowdowns. This patch a
replaces the synchronizations by volatile references. Finally, the changes
remove a doubled reference to another volatile variable. This also simplifies
the generated code by commoning up the corresponding load instruction used in
the fast execution path.
Speedups from this change are uniformly 2x or better.
The proposed improvement and patch was originated by John Rose.
Thanks,
Mike
