RFR: 8177086: java.lang.reflect.AccessibleObject::canAccess should share access cache with internal method ::checkAccess

[Peter Levart]

Hi Alan,

On 03/19/2017 08:03 PM, Alan Bateman wrote:
> On 19/03/2017 17:52, Peter Levart wrote:
>
>> Hi,
>>
>> Module system implementation refresh 2017/2 (JDK-8173393) introduced 
>> new API method AccessibleObject::canAccess which can be used to test 
>> if the caller has access to the reflected member (with possible 
>> target object argument for instance members). The implementation of 
>> this method, after some parameter validation checks, delegates 
>> directly to jdk.internal.reflect.Reflection::verifyMemberAccess. This 
>> is sub-optimal. Co-located internal method 
>> AccessibleObject::checkAccess also delegates to 
>> Reflection::verifyMemberAccess, but it also uses a one-element cache 
>> of access-check decision, which greatly speeds up repeated calls by 
>> the same caller to the same reflected member. The cache could be 
>> shared between those two methods which would improve performance of 
>> code idioms like this:
>>
> When integrating with the cache came up on jigsaw-dev a few work ago 
> then I think Mandy wanted to defer to it to JDK 10. However since 
> canAccess is new then having is be more efficient might be good, it 
> just mightn't be a P1/P2 for RDP2. Can you re-base the patch against 
> jake as has changed this code and then maybe we can figure out whether 
> to pull it into jake?
>
> -Alan

No problem. Here it is (against tip of jake/jdk):

http://cr.openjdk.java.net/~plevart/jdk9-jake/AccessibleObject.canAccess_caching/webrev.02/


This patch changes a little when the stacktrace requested by 
sun.reflect.debugModuleAccessChecks system property is printed. In 
original code it is printed when the access (invocation, get/set) itself 
fails as well as when AccessibleObject.canAccess() returns false. Is 
that what was intended? Patched code only prints when the actual access 
attempt fails and not when canAccess() returns false. If you want 
canAccess() to also print stacktrace, I can add it to canAccess() method.

Tested with java/lang/reflect and all 80 tests pass.

Regards, Peter