Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Commit

Permalink
@StarlarkMethod.trustReturnsValid
Browse files Browse the repository at this point in the history
Summary:
When returning from native functions, Starlark by default checks that returned value is a valid Starlark object (i. e. `String`, `Boolean` or implements `StarlarkValue`). That check is relatively expensive for cheap calls like `dict.get`.

`StarlarkMethod.trustReturnsValid` when used with native function disables this check.

This diff will likely not produce noticeable speedup, but the check is visible in profiler.

Upstream PR: bazelbuild/bazel#13012

Reviewed By: mzlee

fbshipit-source-id: 29e146503a0653a5629dc526c76d64e6f5e5b2f7
  • Loading branch information
stepancheg authored and facebook-github-bot committed Mar 10, 2021
1 parent f557d47 commit 3d4dc6a
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 5 deletions.
1 change: 1 addition & 0 deletions third-party/java/bazel/README.facebook
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ exists here to simplify sync with upstream if we ever do it.
* D26877128 Faster parameter type check for Object
* D26877129 Faster dict.keys
* D26877121 Intern identifiers
* D26877118 @StarlarkMethod.trustReturnsValid
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@
*/
boolean allowReturnNones() default false;

/**
* When true, Starlark interpreter does not validate function returns a valid Starlark runtime object.
*/
boolean trustReturnsValid() default false;

/**
* If true, the StarlarkThread will be passed as an argument of the annotated function. (Thus, the
* annotated method signature must contain StarlarkThread as a parameter. See the interface-level
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ public Iterator<K> iterator() {
named = true,
doc = "The default value to use (instead of None) if the key is not found.")
},
trustReturnsValid = true,
useStarlarkThread = true)
// TODO(adonovan): This method is named get2 as a temporary workaround for a bug in
// StarlarkAnnotations.getStarlarkMethod. The two 'get' methods cause it to get
Expand Down Expand Up @@ -213,6 +214,7 @@ public Object get2(Object key, Object defaultValue, StarlarkThread thread) throw
named = true,
doc = "a default value if the key is absent."),
},
trustReturnsValid = true,
useStarlarkThread = true)
public Object pop(Object key, Object defaultValue, StarlarkThread thread) throws EvalException {
Starlark.checkMutable(this);
Expand Down Expand Up @@ -264,7 +266,8 @@ public Tuple popitem() throws EvalException {
defaultValue = "None",
named = true,
doc = "a default value if the key is absent."),
})
},
trustReturnsValid = true)
public V setdefault(K key, V defaultValue) throws EvalException {
Starlark.checkMutable(this);
Starlark.checkHashable(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ private MethodDescriptor(
boolean extraKeywords,
boolean selfCall,
boolean allowReturnNones,
boolean trustReturnsValid,
boolean useStarlarkThread,
boolean useStarlarkSemantics) {
this.method = method;
Expand All @@ -94,7 +95,8 @@ private MethodDescriptor(
howToHandleReturn = HowToHandleReturn.NULL_TO_NONE;
} else if (StarlarkValue.class.isAssignableFrom(ret)
|| String.class == ret
|| Boolean.class == ret) {
|| Boolean.class == ret
|| trustReturnsValid) {
howToHandleReturn =
allowReturnNones ? HowToHandleReturn.NULL_TO_NONE : HowToHandleReturn.ERROR_ON_NULL;
} else if (ret == int.class) {
Expand Down Expand Up @@ -145,6 +147,7 @@ static MethodDescriptor of(
!annotation.extraKeywords().name().isEmpty(),
annotation.selfCall(),
annotation.allowReturnNones(),
annotation.trustReturnsValid(),
annotation.useStarlarkThread(),
annotation.useStarlarkSemantics());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class MethodLibrary {
+ "or if no arguments are given. "
+ "<pre class=\"language-python\">min(2, 5, 4) == 2\n"
+ "min([5, 6, 3]) == 3</pre>",
extraPositionals = @Param(name = "args", doc = "The elements to be checked."))
extraPositionals = @Param(name = "args", doc = "The elements to be checked."),
trustReturnsValid = true)
public Object min(Sequence<?> args) throws EvalException {
return findExtreme(args, Starlark.ORDERING.reverse());
}
Expand All @@ -54,7 +55,8 @@ public Object min(Sequence<?> args) throws EvalException {
+ "or if no arguments are given. "
+ "<pre class=\"language-python\">max(2, 5, 4) == 5\n"
+ "max([5, 6, 3]) == 6</pre>",
extraPositionals = @Param(name = "args", doc = "The elements to be checked."))
extraPositionals = @Param(name = "args", doc = "The elements to be checked."),
trustReturnsValid = true)
public Object max(Sequence<?> args) throws EvalException {
return findExtreme(args, Starlark.ORDERING);
}
Expand Down Expand Up @@ -625,6 +627,7 @@ public boolean hasattr(Object obj, String name, StarlarkThread thread) throws Ev
"The default value to return in case the struct "
+ "doesn't have an attribute of the given name.")
},
trustReturnsValid = true,
useStarlarkThread = true)
public Object getattr(Object obj, String name, Object defaultValue, StarlarkThread thread)
throws EvalException, InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,8 @@ public int index(Object x, Object start, Object end) throws EvalException {
},
defaultValue = "-1",
doc = "The index of the item.")
})
},
trustReturnsValid = true)
public Object pop(Object i) throws EvalException {
int arg = i == Starlark.NONE ? -1 : Starlark.toInt(i, "i");
int index = EvalUtils.getSequenceIndex(arg, size);
Expand Down

0 comments on commit 3d4dc6a

Please sign in to comment.