Hi Claes,
I have one concern...
645 private static NamedFunction getConstantFunction(int idx) {
646 NamedFunction function = FUNCTIONS[idx];
647 if (function != null) {
648 return function;
649 }
650 return setCachedFunction(idx, makeConstantFunction(idx));
651 }
652
653 private static synchronized NamedFunction
setCachedFunction(int idx, final NamedFunction function) {
654 // Simulate a CAS, to avoid racy duplication of results.
655 NamedFunction prev = FUNCTIONS[idx];
656 if (prev != null) {
657 return prev;
658 }
659 FUNCTIONS[idx] = function;
660 function.resolve();
661 return function;
662 }
Above is a classical double-checked locking idiom, but it is not using
volatile variable to publish the NamedFunction instance. I wonder if
this is safe. Even if the FUNCTIONS[idx] slot was a volatile variable,
you would publish new instance before resolving it. Is it OK to publish
unresolved NamedFunction(s)? There is a NamedFunction.resolvedHandle()
instance method that makes sure NamedFunction is resolved before
returning a MethodHandle, but there are also usages of dereferencing
NamedFunction.resolvedHandle field directly in code. Are you sure that
such unresolved or almost resolved instance of NamedFunction is never
used in such places where NamedFunction.resolvedHandle field is
dereferenced directly?
In original code those NamedFunctions were resolved in static
initializer so they were published properly.
Regards, Peter
On 11/12/2015 04:55 PM, Claes Redestad wrote:
On 2015-11-12 14:47, Paul Sandoz wrote:
On 11 Nov 2015, at 15:32, Claes Redestad <claes.redes...@oracle.com>
wrote:
Paul,
On 2015-11-10 11:55, Paul Sandoz wrote:
DirectMethodHandle
—
682 private static @Stable NamedFunction[] FUNCTIONS = new
NamedFunction[NF_LIMIT];
Invokers
—
442 private static @Stable NamedFunction[] FUNCTIONS = new
NamedFunction[NF_LIMIT];
MethodHandleImpl
—
1627 private static @Stable NamedFunction[] FUNCTIONS = new
NamedFunction[NF_LIMIT];
To be complete you could add “final”, thus it makes it clear that
@Stable refers specifically to the array element.
Paul.
Thanks for having a look and catching this:
http://cr.openjdk.java.net/~redestad/8142334/webrev.03
- added final keyword to FUNCTIONS and HANDLES
- added @Stable to ARRAYS, FILL_ARRAYS, and FILL_ARRAY_TO_RIGHT
MethodHandleImpl.java
—
1413 private static final @Stable MethodHandle[] FILL_ARRAYS =
new MethodHandle[FILL_ARRAYS_COUNT + 1];
1414
1415 private static MethodHandle getFillArray(int count) {
1416 assert (count > 0 && count <= FILL_ARRAYS_COUNT);
Why FILL_ARRAYS_COUNT + 1 rather than FILL_ARRAYS_COUNT?
Based on the previous code I would have expected the bounds to be:
0 < count < FILL_ARRAYS_COUNT
Paul.
Yes. Updated http://cr.openjdk.java.net/~redestad/8142334/webrev.03
in-place.
/Claes
