diff --git a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java index 4a7503677d9..649e521875c 100644 --- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java +++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java @@ -542,7 +542,7 @@ public abstract class ScriptFunction extends ScriptObject { } //else just fall through and link as ordinary function or unstable apply final int programPoint = NashornCallSiteDescriptor.isOptimistic(desc) ? NashornCallSiteDescriptor.getProgramPoint(desc) : INVALID_PROGRAM_POINT; - final CompiledFunction cf = data.getBestInvoker(type, programPoint, scope); + final CompiledFunction cf = data.getBestInvoker(type, scope); final GuardedInvocation bestInvoker = new GuardedInvocation( cf.createInvoker(type.returnType(), programPoint), diff --git a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java index 00cd9762c7e..13dbee9cd8f 100644 --- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java +++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java @@ -32,12 +32,6 @@ import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; -import java.lang.ref.Reference; -import java.lang.ref.WeakReference; -import java.util.Collections; -import java.util.Map; -import java.util.WeakHashMap; - import jdk.nashorn.internal.runtime.linker.LinkerCallSite; /** @@ -53,9 +47,6 @@ public abstract class ScriptFunctionData { assert MAX_ARITY < 256; } - private static final Map> GENERIC_INVOKERS = Collections.synchronizedMap(new WeakHashMap>()); - private static final Map> GENERIC_CONSTRUCTORS = Collections.synchronizedMap(new WeakHashMap>()); - /** Name of the function or "" for anonymous functions */ protected final String name; @@ -71,6 +62,13 @@ public abstract class ScriptFunctionData { // value, the function might still be capable of receiving variable number of arguments, see isVariableArity. private int arity; + /** + * A pair of method handles used for generic invoker and constructor. Field is volatile as it can be initialized by + * multiple threads concurrently, but we still tolerate a race condition in it as all values stored into it are + * idempotent. + */ + private volatile GenericInvokers genericInvokers; + private static final MethodHandle BIND_VAR_ARGS = findOwnMH("bindVarArgs", Object[].class, Object[].class, Object[].class); /** Is this a strict mode function? */ @@ -221,7 +219,7 @@ public abstract class ScriptFunctionData { * @return guarded invocation with method handle to best invoker and potentially a switch point guarding optimistic * assumptions. */ - final CompiledFunction getBestInvoker(final MethodType callSiteType, final int callerProgramPoint, final ScriptObject runtimeScope) { + final CompiledFunction getBestInvoker(final MethodType callSiteType, final ScriptObject runtimeScope) { final CompiledFunction cf = getBest(callSiteType, runtimeScope); assert cf != null; return cf; @@ -255,16 +253,14 @@ public abstract class ScriptFunctionData { * @return generic invoker of this script function */ final MethodHandle getGenericInvoker(final ScriptObject runtimeScope) { - MethodHandle invoker; - final Reference ref = GENERIC_INVOKERS.get(this); - if(ref != null) { - invoker = ref.get(); - if(invoker != null) { - return invoker; - } + // This method has race conditions both on genericsInvoker and genericsInvoker.invoker, but even if invoked + // concurrently, they'll create idempotent results, so it doesn't matter. We could alternatively implement this + // using java.util.concurrent.AtomicReferenceFieldUpdater, but it's hardly worth it. + final GenericInvokers lgenericInvokers = ensureGenericInvokers(); + MethodHandle invoker = lgenericInvokers.invoker; + if(invoker == null) { + lgenericInvokers.invoker = invoker = createGenericInvoker(runtimeScope); } - invoker = createGenericInvoker(runtimeScope); - GENERIC_INVOKERS.put(this, new WeakReference<>(invoker)); return invoker; } @@ -273,16 +269,14 @@ public abstract class ScriptFunctionData { } final MethodHandle getGenericConstructor(final ScriptObject runtimeScope) { - MethodHandle constructor; - final Reference ref = GENERIC_CONSTRUCTORS.get(this); - if(ref != null) { - constructor = ref.get(); - if(constructor != null) { - return constructor; - } + // This method has race conditions both on genericsInvoker and genericsInvoker.constructor, but even if invoked + // concurrently, they'll create idempotent results, so it doesn't matter. We could alternatively implement this + // using java.util.concurrent.AtomicReferenceFieldUpdater, but it's hardly worth it. + final GenericInvokers lgenericInvokers = ensureGenericInvokers(); + MethodHandle constructor = lgenericInvokers.constructor; + if(constructor == null) { + lgenericInvokers.constructor = constructor = createGenericConstructor(runtimeScope); } - constructor = createGenericConstructor(runtimeScope); - GENERIC_CONSTRUCTORS.put(this, new WeakReference<>(constructor)); return constructor; } @@ -290,6 +284,14 @@ public abstract class ScriptFunctionData { return makeGenericMethod(getGeneric(runtimeScope).createComposableConstructor()); } + private GenericInvokers ensureGenericInvokers() { + GenericInvokers lgenericInvokers = genericInvokers; + if(lgenericInvokers == null) { + genericInvokers = lgenericInvokers = new GenericInvokers(); + } + return lgenericInvokers; + } + /** * Returns the best function for the specified call site type. * @param callSiteType The call site type. Call site types are expected to have the form @@ -781,4 +783,14 @@ public abstract class ScriptFunctionData { private static MethodHandle findOwnMH(final String name, final Class rtype, final Class... types) { return MH.findStatic(MethodHandles.lookup(), ScriptFunctionData.class, name, MH.type(rtype, types)); } + + /** + * This class is used to hold the generic invoker and generic constructor pair. It is structured in this way since + * most functions will never use them, so this way ScriptFunctionData only pays storage cost for one null reference + * to the GenericInvokers object, instead of two null references for the two method handles. + */ + private static final class GenericInvokers { + volatile MethodHandle invoker; + volatile MethodHandle constructor; + } }