diff --git a/src/main/java/net/starlark/java/eval/BuiltinFunction.java b/src/main/java/net/starlark/java/eval/BuiltinFunction.java index aab7a8d6ea2f89..c7c4b5eff3c13e 100644 --- a/src/main/java/net/starlark/java/eval/BuiltinFunction.java +++ b/src/main/java/net/starlark/java/eval/BuiltinFunction.java @@ -128,9 +128,11 @@ public String toString() { * @param thread the Starlark thread for the call * @param loc the location of the call expression, or BUILTIN for calls from Java * @param desc descriptor for the StarlarkMethod-annotated method - * @param positional a list of positional arguments + * @param positional an array of positional arguments; as an optimization, in simple cases, this + * array may be reused as the method's return value * @param named a list of named arguments, as alternating Strings/Objects. May contain dups. - * @return the array of arguments which may be passed to {@link MethodDescriptor#call} + * @return the array of arguments which may be passed to {@link MethodDescriptor#call}. It is + * unsafe to mutate the returned array. * @throws EvalException if the given set of arguments are invalid for the given method. For * example, if any arguments are of unexpected type, or not all mandatory parameters are * specified by the user @@ -156,6 +158,16 @@ private Object[] getArgumentVector( ParamDescriptor[] parameters = desc.getParameters(); + // Fast case: we only accept positionals and can reuse `positional` as the Java args vector. + // Note that StringModule methods, which are treated specially below, will never match this case + // since their `self` parameter is restricted to String and thus + // isPositionalsReusableAsCallArgsVectorIfArgumentCountValid() would be false. + if (desc.isPositionalsReusableAsJavaArgsVectorIfArgumentCountValid() + && positional.length == parameters.length + && named.length == 0) { + return positional; + } + // Allocate argument vector. int n = parameters.length; if (desc.acceptsExtraArgs()) { diff --git a/src/main/java/net/starlark/java/eval/MethodDescriptor.java b/src/main/java/net/starlark/java/eval/MethodDescriptor.java index 53c4f5265dbceb..627c3a846d7d96 100644 --- a/src/main/java/net/starlark/java/eval/MethodDescriptor.java +++ b/src/main/java/net/starlark/java/eval/MethodDescriptor.java @@ -14,6 +14,8 @@ package net.starlark.java.eval; +import static java.util.Arrays.stream; + import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.errorprone.annotations.CheckReturnValue; @@ -46,6 +48,7 @@ final class MethodDescriptor { private final boolean allowReturnNones; private final boolean useStarlarkThread; private final boolean useStarlarkSemantics; + private final boolean positionalsReusableAsJavaArgsVectorIfArgumentCountValid; private enum HowToHandleReturn { NULL_TO_NONE, // any Starlark value; null -> None @@ -100,6 +103,19 @@ private MethodDescriptor( } else { howToHandleReturn = HowToHandleReturn.FROM_JAVA; } + + this.positionalsReusableAsJavaArgsVectorIfArgumentCountValid = + !extraKeywords + && !extraPositionals + && !useStarlarkSemantics + && !useStarlarkThread + && stream(parameters).allMatch(MethodDescriptor::paramUsableAsPositionalWithoutChecks); + } + + private static boolean paramUsableAsPositionalWithoutChecks(ParamDescriptor param) { + return param.isPositional() + && param.disabledByFlag() == null + && param.getAllowedClasses() == null; } /** Returns the StarlarkMethod annotation corresponding to this method. */ @@ -287,4 +303,18 @@ String getDoc() { boolean isSelfCall() { return selfCall; } + + /** + * Returns true if we may directly reuse the Starlark positionals vector as the Java {@code args} + * vector passed to {@link #call} as long as the Starlark call was made with a valid number of + * arguments. + * + *

More precisely, this means that we do not need to insert extra values into the args vector + * (such as ones corresponding to {@code *args}, {@code **kwargs}, or {@code self} in Starlark), + * and all Starlark parameters are simple positional parameters which cannot be disabled by a flag + * and do not require type checking. + */ + boolean isPositionalsReusableAsJavaArgsVectorIfArgumentCountValid() { + return positionalsReusableAsJavaArgsVectorIfArgumentCountValid; + } } diff --git a/src/test/java/net/starlark/java/eval/MethodLibraryTest.java b/src/test/java/net/starlark/java/eval/MethodLibraryTest.java index 0e76b265fe7a48..f2204aee53676e 100644 --- a/src/test/java/net/starlark/java/eval/MethodLibraryTest.java +++ b/src/test/java/net/starlark/java/eval/MethodLibraryTest.java @@ -22,7 +22,9 @@ import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import java.util.List; +import net.starlark.java.annot.Param; import net.starlark.java.annot.StarlarkBuiltin; +import net.starlark.java.annot.StarlarkMethod; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -772,6 +774,29 @@ ev.new Scenario() "','.join(elements=['foo', 'bar'])"); } + @StarlarkBuiltin(name = "named_only", doc = "") + static final class NamedOnly implements StarlarkValue { + @StarlarkMethod( + name = "foo", + documented = false, + parameters = { + @Param(name = "a"), + @Param(name = "b", named = true, defaultValue = "None", positional = false), + }) + public Object foo(Object a, Object b) { + return a; + } + } + + @Test + public void testNamedOnlyArgument() throws Exception { + ev.new Scenario() + .update("named_only", new NamedOnly()) + .testIfErrorContains( + "foo() accepts no more than 1 positional argument but got 2", + "named_only.foo([1, 2, 3], int)"); + } + @Test public void testStringJoinRequiresStrings() throws Exception { ev.new Scenario() diff --git a/src/test/java/net/starlark/java/eval/testdata/bench_call.star b/src/test/java/net/starlark/java/eval/testdata/bench_call.star new file mode 100644 index 00000000000000..5c0661947868de --- /dev/null +++ b/src/test/java/net/starlark/java/eval/testdata/bench_call.star @@ -0,0 +1,21 @@ +def bench_call_type(b): + for _ in range(b.n): + type(1) + +def bench_call_list_append(b): + for _ in range(b.n): + [].append("foo") + +def bench_call_dict_get(b): + d = {"foo": "bar"} + for _ in range(b.n): + d.get("baz") + +def bench_call_dict_get_none(b): + d = {"foo": "bar"} + for _ in range(b.n): + d.get("baz", None) + +def bench_call_bool(b): + for _ in range(b.n): + bool()