Paul,

Thanks for the feedback!

Updated version:
  http://cr.openjdk.java.net/~vlivanov/8059877/webrev.01/

http://cr.openjdk.java.net/~vlivanov/8059877/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8059877


Generally looks ok.

- MethodHandleImpl

  786              if (wrapper.unblock()) {
  787                  // Reached invocation threshold. Replace 
counting/blocking wrapper with a reinvoker.
  788                  wrapper.isActivated = false;

If unblock() returns true then isActivated is already false.
Fixed.

At the expense of another box you could use an AtomicBoolean with compareAndSet 
if you want to really guarantee at most one unblocking, although the box can be 
removed if the boolean is changed to 'int' and Unsafe is used to CAS.
I considered that, but I decided not to go that route. GuardWithTest is a very common combinator, so footprint overhead could be noticeable. There's no need to guarantee uniqueness of unblocking operation. The operation is idempotent, so no problems performing it multiple times. What I try to achieve with the flag is avoid pathological situation when some thread continuously updates the form.

I dunno if strengthening the visibility of MethodHandle.form by stamping in a 
memory fence after the following will help

  792                  wrapper.updateForm(lform);

e.g. using Unsafe.fullFence.
I think the fence should be there. It won't help guarantee the update is visible everywhere though. But it is expected.

Perhaps isUnblocked is a better name than isActivated since there is no need to 
set the initial value and it tracks the same value as that returned from 
unblock?
Done.

Also while on the subject of naming perhaps consider changing 
MethodTypeForm.LF_DELEGATE_COUNTING to ...LF_DELEGATE_BLOCK_INLINING?
Done.

When DONT_INLINE_THRESHOLD > 0 and DONT_INLINE_THRESHOLD == COMPILE_THRESHOLD 
will that trigger both compilation of a BlockInliningWrapper's form and 
unblocking? I guess there is some fuzziness depending on concurrent execution.
No problems expected in this scenario - COMPILE_THRESHOLD updates LambdaForm entry point and unblocking operates on a method handle.

I am just wondering if there is any point compiling a BlockInliningWrapper's 
form if DONT_INLINE_THRESHOLD is expected to be ~ the same as 
COMPILE_THRESHOLD. Probably not terribly important especially if the 
JIT-compiler control approach replaces it.
I don't see any value in keeping BlockInliningWrapper interpreted.
It would allow to avoid LambdaForm.forceInline flag, but there should be a special case to skip compilation (in LambdaForm::checkInvocationCounter()). Moreover, if we get rid of LambdaForm interpreter, we will need to precompile BlockInliningWrapper again.

Best regards,
Vladimir Ivanov

Reply via email to