On Fri, 6 Nov 2020 00:24:14 GMT, Hui Shi <h...@openjdk.org> wrote:

>> If we are changing NativeMethodAccessorImpl.invoke then we should probably 
>> do NativeConstructorAccessorImpl.newInstance at the same time. Also 
>> wondering if we should, while in the area, add "return acc.invoke(obj, 
>> args)" after setting the delegate so that it invokes the newly generated 
>> accessor.
>> 
>> Are there resource or other cases that you have observed where 
>> generateMethod fails and then succeeds in a subsequent call?
>> 
>> @cl4es Do you know of any startup tests that might be sensitive to the eager 
>> creating of a VarHandle?
>> 
>> I agree with @shipilev to test before the CAS.
>
>> If we are changing NativeMethodAccessorImpl.invoke then we should probably 
>> do NativeConstructorAccessorImpl.newInstance at the same time. 
> 
> Yes, NativeConstructorAccessorImpl should also apply this change.
> 
>> Also wondering if we should, while in the area, add "return acc.invoke(obj, 
>> args)" after setting the delegate so that it invokes the newly generated 
>> accessor.
>> 
> 
> Agree,  I see no harm to invoke generated method when it is aviable. 
> Should leave this to another patch?
> 
>> Are there resource or other cases that you have observed where 
>> generateMethod fails and then succeeds in a subsequent call?
>> 
> I have not seen exception/error happen in generate method yet.  But in case 
> it fails in some ways, try - catch - reset is added to make sure behavior is 
> same before/after with this change.
> 
>> @cl4es Do you know of any startup tests that might be sensitive to the eager 
>> creating of a VarHandle?
>> 
>> I agree with @shipilev to test before the CAS.
> 
> @AlanBateman 
> Thanks for you comments!

Are there any benchmarks to compare this accessor with the previous version in 
the presumably common case where there is no or very little contention? Edit to 
clarify: it is stated as "trivial" is this also measured somewhere?

-------------

PR: https://git.openjdk.java.net/jdk/pull/1070

Reply via email to