From 0596e1b0136f55ebe480143041da96cf0b2db071 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 24 Jan 2023 22:18:16 -0800 Subject: [PATCH] Optimise Starlark providers for non-sparse values This doesn't bring immediate improvement for current BUILD, where native providers are predominant. The improvement is although necessary for the rewrite of the most common native providers to Starlark: ProtoInfo, JavaInfo, CcInfo. PiperOrigin-RevId: 504464672 Change-Id: I43bc08a6218291edd25f408c5a68c1941ba82675 --- .../build/lib/packages/StarlarkInfo.java | 313 +----------------- .../lib/packages/StarlarkInfoNoSchema.java | 296 +++++++++++++++++ .../lib/packages/StarlarkInfoWithMessage.java | 14 +- .../lib/packages/StarlarkInfoWithSchema.java | 156 +++++++++ .../build/lib/packages/StarlarkProvider.java | 21 +- .../build/lib/packages/StarlarkInfoTest.java | 8 +- 6 files changed, 492 insertions(+), 316 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoNoSchema.java create mode 100644 src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithSchema.java diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java index ac0ed5a8bed5bf..2706d6763b80a3 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfo.java @@ -1,4 +1,4 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. +// Copyright 2023 The Bazel Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -14,319 +14,36 @@ package com.google.devtools.build.lib.packages; -import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableCollection; -import com.google.common.collect.ImmutableList; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; import java.util.Map; import javax.annotation.Nullable; -import net.starlark.java.eval.EvalException; import net.starlark.java.eval.HasBinary; -import net.starlark.java.eval.Starlark; import net.starlark.java.syntax.Location; -import net.starlark.java.syntax.TokenKind; -/** An struct-like Info (provider instance) for providers defined in Starlark. */ -public class StarlarkInfo extends StructImpl implements HasBinary { - private final Provider provider; +/** Superclass (provider instance) for providers defined in Starlark. */ +public abstract class StarlarkInfo extends StructImpl implements HasBinary { - // For a n-element info, the table contains n key strings, sorted, - // followed by the n corresponding legal Starlark values. - private final Object[] table; - - // TODO(adonovan): restrict type of provider to StarlarkProvider? - // Do we ever need StarlarkInfos of BuiltinProviders? Such BuiltinProviders could - // be moved to Starlark using bzl builtins injection. - // Alternatively: what about this implementation is specific to StarlarkProvider? - // It's really just a "generic" or "dynamic" representation of a struct, - // analogous to reflection versus generated message classes in the protobuf world. - // The efficient table algorithms would be a nice addition to the Starlark - // interpreter, to allow other clients to define their own fast structs - // (or to define a standard one). See also comments at Info about upcoming clean-ups. - StarlarkInfo(Provider provider, Object[] table, @Nullable Location loc) { - super(loc); - this.provider = provider; - this.table = table; - } - - @Override - public Provider getProvider() { - return provider; - } - - // Converts a map to a table of sorted keys followed by corresponding values. - static Object[] toTable(Map values) { - int n = values.size(); - Object[] table = new Object[n + n]; - int i = 0; - for (Map.Entry e : values.entrySet()) { - table[i] = e.getKey(); - table[n + i] = Starlark.checkValid(e.getValue()); - i++; - } - // Sort keys, permuting values in parallel. - if (n > 1) { - sortPairs(table, 0, n - 1); - } - return table; - } - - /** - * Constructs a StarlarkInfo from an array of alternating key/value pairs as provided by - * Starlark.fastcall. Checks that each key is provided at most once, and is defined by the - * optional schema, which must be sorted. This optimized zero-allocation function exists solely - * for the StarlarkProvider constructor. - */ - static StarlarkInfo createFromNamedArgs( - Provider provider, Object[] table, @Nullable ImmutableList schema, Location loc) - throws EvalException { - // Permute fastcall form (k, v, ..., k, v) into table form (k, k, ..., v, v). - permute(table); - - int n = table.length >> 1; // number of K/V pairs - - // Sort keys, permuting values in parallel. - if (n > 1) { - sortPairs(table, 0, n - 1); - } - - // Check for duplicate keys, which are now adjacent. - for (int i = 0; i < n - 1; i++) { - if (table[i].equals(table[i + 1])) { - throw Starlark.errorf( - "got multiple values for parameter %s in call to instantiate provider %s", - table[i], provider.getPrintableName()); - } - } - - // Check that schema is a superset of the table's keys. - if (schema != null) { - List unexpected = unexpectedKeys(schema, table, n); - if (unexpected != null) { - throw Starlark.errorf( - "got unexpected field%s '%s' in call to instantiate provider %s", - unexpected.size() > 1 ? "s" : "", - Joiner.on("', '").join(unexpected), - provider.getPrintableName()); - } - } - - return new StarlarkInfo(provider, table, loc); - } - - // Permutes array elements from alternating keys/values form, - // (as used by fastcall's named array) into keys-then-corresponding-values form, - // as used by StarlarkInfo.table. - // The permutation preserves the key/value association but not the order of keys. - static void permute(Object[] named) { - int n = named.length >> 1; // number of K/V pairs - - // Thanks to Murali Ganapathy for the algorithm. - // See https://play.golang.org/p/QOKnrj_bIwk. - // - // i and j are the indices bracketing successive pairs of cells, - // working from the outside to the middle. - // - // i j - // [KV]KVKVKVKVKVKV[KV] - // i j - // KK[KV]KVKVKVKV[KV]VV - // i j - // KKKK[KV]KVKV[KV]VVVV - // etc... - for (int i = 0; i < n - 1; i += 2) { - int j = named.length - i; - // rotate two pairs [KV]...[kv] -> [Kk]...[vV] - Object tmp = named[i + 1]; - named[i + 1] = named[j - 2]; - named[j - 2] = named[j - 1]; - named[j - 1] = tmp; - } - // reverse lower half containing keys: [KkvV] -> [kKvV] - for (int i = 0; i < n >> 1; i++) { - Object tmp = named[n - 1 - i]; - named[n - 1 - i] = named[i]; - named[i] = tmp; - } - } - - // Sorts non-empty slice a[lo:hi] (inclusive) in place. - // Elements a[n:2n) are permuted the same way as a[0:n), - // where n = a.length / 2. The lower half must be strings. - // Precondition: 0 <= lo <= hi < n. - static void sortPairs(Object[] a, int lo, int hi) { - String pivot = (String) a[lo + (hi - lo) / 2]; - - int i = lo; - int j = hi; - while (i <= j) { - while (((String) a[i]).compareTo(pivot) < 0) { - i++; - } - while (((String) a[j]).compareTo(pivot) > 0) { - j--; - } - if (i <= j) { - int n = a.length >> 1; - swap(a, i, j); - swap(a, i + n, j + n); - i++; - j--; - } - } - if (lo < j) { - sortPairs(a, lo, j); - } - if (i < hi) { - sortPairs(a, i, hi); - } - } - - private static void swap(Object[] a, int i, int j) { - Object tmp = a[i]; - a[i] = a[j]; - a[j] = tmp; - } - - // Returns the list of keys in table[0:n) not defined by the schema, - // or null on success. - // Allocates no memory on success. - // Both table[0:n) and schema are sorted lists of strings. - @Nullable - private static List unexpectedKeys(ImmutableList schema, Object[] table, int n) { - int si = 0; - List unexpected = null; - table: - for (int ti = 0; ti < n; ti++) { - String t = (String) table[ti]; - while (si < schema.size()) { - String s = schema.get(si++); - int cmp = s.compareTo(t); - if (cmp == 0) { - // table key matches schema - continue table; - } else if (cmp > 0) { - // table contains unexpected key - if (unexpected == null) { - unexpected = new ArrayList<>(); - } - unexpected.add(t); - } else { - // skip over schema key not provided by table - } - } - if (unexpected == null) { - unexpected = new ArrayList<>(); - } - unexpected.add(t); - } - return unexpected; - } - - @Override - public ImmutableCollection getFieldNames() { - // TODO(adonovan): opt: can we avoid allocating three objects? - @SuppressWarnings("unchecked") - List keys = (List) (List) Arrays.asList(table).subList(0, table.length / 2); - return ImmutableList.copyOf(keys); - } - - @Override - public boolean isImmutable() { - // If the provider is not yet exported, the hash code of the object is subject to change. - if (!provider.isExported()) { - return false; - } - for (int i = table.length / 2; i < table.length; i++) { - if (!Starlark.isImmutable(table[i])) { - return false; - } - } - return true; - } - - @Nullable - @Override - public Object getValue(String name) { - int n = table.length / 2; - int i = Arrays.binarySearch(table, 0, n, name); - if (i < 0) { - return null; - } - return table[n + i]; + StarlarkInfo(@Nullable Location location) { + super(location); } /** * Creates a schemaless provider instance with the given provider type and field values. * - *

{@code loc} is the creation location for this instance. Built-in provider instances may use - * {@link Location#BUILTIN}, which is the default if null. + * @param provider A {@code Provider} without a schema. {@code StarlarkProvider} with a schema is + * not supported by this call. + * @param values the field values + * @param loc the creation location for this instance. Built-in provider instances may use {@link + * Location#BUILTIN}, which is the default if null. */ public static StarlarkInfo create( Provider provider, Map values, @Nullable Location loc) { - return new StarlarkInfo(provider, toTable(values), loc); + return StarlarkInfoNoSchema.createSchemaless(provider, values, loc); } + // Relax visibility to public. getValue() is widely used to directly access fields from native + // rule logic. Compared to Starlark.getattr(), it also avoids the need for the caller to pass a + // Semantics. @Nullable @Override - public StarlarkInfo binaryOp(TokenKind op, Object that, boolean thisLeft) throws EvalException { - if (op == TokenKind.PLUS && that instanceof StarlarkInfo) { - return thisLeft - ? plus(this, (StarlarkInfo) that) // - : plus((StarlarkInfo) that, this); - } - return null; - } - - private static StarlarkInfo plus(StarlarkInfo x, StarlarkInfo y) throws EvalException { - Provider xprov = x.provider; - Provider yprov = y.provider; - if (!xprov.equals(yprov)) { - throw Starlark.errorf( - "Cannot use '+' operator on instances of different providers (%s and %s)", - xprov.getPrintableName(), yprov.getPrintableName()); - } - - // ztable = merge(x.table, y.table) - int xsize = x.table.length / 2; - int ysize = y.table.length / 2; - int zsize = xsize + ysize; - Object[] ztable = new Object[zsize + zsize]; - int xi = 0; - int yi = 0; - int zi = 0; - while (xi < xsize && yi < ysize) { - String xk = (String) x.table[xi]; - String yk = (String) y.table[yi]; - int cmp = xk.compareTo(yk); - if (cmp < 0) { - ztable[zi] = xk; - ztable[zi + zsize] = x.table[xi + xsize]; - xi++; - } else if (cmp > 0) { - ztable[zi] = yk; - ztable[zi + zsize] = y.table[yi + ysize]; - yi++; - } else { - throw Starlark.errorf("cannot add struct instances with common field '%s'", xk); - } - zi++; - } - while (xi < xsize) { - ztable[zi] = x.table[xi]; - ztable[zi + zsize] = x.table[xi + xsize]; - xi++; - zi++; - } - while (yi < ysize) { - ztable[zi] = y.table[yi]; - ztable[zi + zsize] = y.table[yi + ysize]; - yi++; - zi++; - } - - return new StarlarkInfo(xprov, ztable, Location.BUILTIN); - } + public abstract Object getValue(String name); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoNoSchema.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoNoSchema.java new file mode 100644 index 00000000000000..ea20bed5f8dc7e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoNoSchema.java @@ -0,0 +1,296 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.packages; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableCollection; +import com.google.common.collect.ImmutableList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Starlark; +import net.starlark.java.syntax.Location; +import net.starlark.java.syntax.TokenKind; + +/** + * A struct-like Info (provider instance) for providers defined in Starlark that don't have a + * schema. + */ +public class StarlarkInfoNoSchema extends StarlarkInfo { + private final Provider provider; + + // For a n-element info, the table contains n key strings, sorted, + // followed by the n corresponding legal Starlark values. + private final Object[] table; + + // TODO(adonovan): restrict type of provider to StarlarkProvider? + // Do we ever need StarlarkInfos of BuiltinProviders? Such BuiltinProviders could + // be moved to Starlark using bzl builtins injection. + // Alternatively: what about this implementation is specific to StarlarkProvider? + // It's really just a "generic" or "dynamic" representation of a struct, + // analogous to reflection versus generated message classes in the protobuf world. + // The efficient table algorithms would be a nice addition to the Starlark + // interpreter, to allow other clients to define their own fast structs + // (or to define a standard one). See also comments at Info about upcoming clean-ups. + private StarlarkInfoNoSchema(Provider provider, Object[] table, @Nullable Location loc) { + super(loc); + this.provider = provider; + this.table = table; + } + + StarlarkInfoNoSchema(Provider provider, Map values, @Nullable Location loc) { + super(loc); + this.provider = provider; + this.table = toTable(values); + } + + @Override + public Provider getProvider() { + return provider; + } + + /** + * Creates a schemaless provider instance with the given provider type and field values. + * + * @param provider A {@code Provider} without a schema. {@code StarlarkProvider} with a schema is + * not supported by this call. + * @param values the field values + * @param loc the creation location for this instance. Built-in provider instances may use {@link + * Location#BUILTIN}, which is the default if null. + */ + static StarlarkInfo createSchemaless( + Provider provider, Map values, @Nullable Location loc) { + Preconditions.checkArgument( + !(provider instanceof StarlarkProvider) + || ((StarlarkProvider) provider).getFields() == null); + return new StarlarkInfoNoSchema(provider, values, loc); + } + + // Converts a map to a table of sorted keys followed by corresponding values. + private static Object[] toTable(Map values) { + int n = values.size(); + Object[] table = new Object[n + n]; + int i = 0; + for (Map.Entry e : values.entrySet()) { + table[i] = e.getKey(); + table[n + i] = Starlark.checkValid(e.getValue()); + i++; + } + // Sort keys, permuting values in parallel. + if (n > 1) { + sortPairs(table, 0, n - 1); + } + return table; + } + + /** + * Constructs a StarlarkInfo from an array of alternating key/value pairs as provided by + * Starlark.fastcall. Checks that each key is provided at most once. This optimized + * zero-allocation function exists solely for the StarlarkProvider constructor. + */ + static StarlarkInfo createFromNamedArgs(StarlarkProvider provider, Object[] table, Location loc) + throws EvalException { + // Permute fastcall form (k, v, ..., k, v) into table form (k, k, ..., v, v). + permute(table); + + int n = table.length >> 1; // number of K/V pairs + + // Sort keys, permuting values in parallel. + if (n > 1) { + sortPairs(table, 0, n - 1); + } + + // Check for duplicate keys, which are now adjacent. + for (int i = 0; i < n - 1; i++) { + if (table[i].equals(table[i + 1])) { + throw Starlark.errorf( + "got multiple values for parameter %s in call to instantiate provider %s", + table[i], provider.getPrintableName()); + } + } + + return new StarlarkInfoNoSchema(provider, table, loc); + } + + // Permutes array elements from alternating keys/values form, + // (as used by fastcall's named array) into keys-then-corresponding-values form, + // as used by StarlarkInfo.table. + // The permutation preserves the key/value association but not the order of keys. + static void permute(Object[] named) { + int n = named.length >> 1; // number of K/V pairs + + // Thanks to Murali Ganapathy for the algorithm. + // See https://play.golang.org/p/QOKnrj_bIwk. + // + // i and j are the indices bracketing successive pairs of cells, + // working from the outside to the middle. + // + // i j + // [KV]KVKVKVKVKVKV[KV] + // i j + // KK[KV]KVKVKVKV[KV]VV + // i j + // KKKK[KV]KVKV[KV]VVVV + // etc... + for (int i = 0; i < n - 1; i += 2) { + int j = named.length - i; + // rotate two pairs [KV]...[kv] -> [Kk]...[vV] + Object tmp = named[i + 1]; + named[i + 1] = named[j - 2]; + named[j - 2] = named[j - 1]; + named[j - 1] = tmp; + } + // reverse lower half containing keys: [KkvV] -> [kKvV] + for (int i = 0; i < n >> 1; i++) { + Object tmp = named[n - 1 - i]; + named[n - 1 - i] = named[i]; + named[i] = tmp; + } + } + + // Sorts non-empty slice a[lo:hi] (inclusive) in place. + // Elements a[n:2n) are permuted the same way as a[0:n), + // where n = a.length / 2. The lower half must be strings. + // Precondition: 0 <= lo <= hi < n. + static void sortPairs(Object[] a, int lo, int hi) { + String pivot = (String) a[lo + (hi - lo) / 2]; + + int i = lo; + int j = hi; + while (i <= j) { + while (((String) a[i]).compareTo(pivot) < 0) { + i++; + } + while (((String) a[j]).compareTo(pivot) > 0) { + j--; + } + if (i <= j) { + int n = a.length >> 1; + swap(a, i, j); + swap(a, i + n, j + n); + i++; + j--; + } + } + if (lo < j) { + sortPairs(a, lo, j); + } + if (i < hi) { + sortPairs(a, i, hi); + } + } + + private static void swap(Object[] a, int i, int j) { + Object tmp = a[i]; + a[i] = a[j]; + a[j] = tmp; + } + + @Override + public ImmutableCollection getFieldNames() { + // TODO(adonovan): opt: can we avoid allocating three objects? + @SuppressWarnings("unchecked") + List keys = (List) (List) Arrays.asList(table).subList(0, table.length / 2); + return ImmutableList.copyOf(keys); + } + + @Override + public boolean isImmutable() { + // If the provider is not yet exported, the hash code of the object is subject to change. + if (!provider.isExported()) { + return false; + } + for (int i = table.length / 2; i < table.length; i++) { + if (!Starlark.isImmutable(table[i])) { + return false; + } + } + return true; + } + + @Nullable + @Override + public Object getValue(String name) { + int n = table.length / 2; + int i = Arrays.binarySearch(table, 0, n, name); + if (i < 0) { + return null; + } + return table[n + i]; + } + + @Nullable + @Override + public StarlarkInfo binaryOp(TokenKind op, Object that, boolean thisLeft) throws EvalException { + if (op == TokenKind.PLUS && that instanceof StarlarkInfo) { + final Provider thatProvider = ((StarlarkInfo) that).getProvider(); + if (!provider.equals(thatProvider)) { + throw Starlark.errorf( + "Cannot use '+' operator on instances of different providers (%s and %s)", + provider.getPrintableName(), thatProvider.getPrintableName()); + } + Preconditions.checkArgument(that instanceof StarlarkInfoNoSchema); + return thisLeft + ? plus(this, (StarlarkInfoNoSchema) that) // + : plus((StarlarkInfoNoSchema) that, this); + } + return null; + } + + private static StarlarkInfo plus(StarlarkInfoNoSchema x, StarlarkInfoNoSchema y) + throws EvalException { + // ztable = merge(x.table, y.table) + int xsize = x.table.length / 2; + int ysize = y.table.length / 2; + int zsize = xsize + ysize; + Object[] ztable = new Object[zsize + zsize]; + int xi = 0; + int yi = 0; + int zi = 0; + while (xi < xsize && yi < ysize) { + String xk = (String) x.table[xi]; + String yk = (String) y.table[yi]; + int cmp = xk.compareTo(yk); + if (cmp < 0) { + ztable[zi] = xk; + ztable[zi + zsize] = x.table[xi + xsize]; + xi++; + } else if (cmp > 0) { + ztable[zi] = yk; + ztable[zi + zsize] = y.table[yi + ysize]; + yi++; + } else { + throw Starlark.errorf("cannot add struct instances with common field '%s'", xk); + } + zi++; + } + while (xi < xsize) { + ztable[zi] = x.table[xi]; + ztable[zi + zsize] = x.table[xi + xsize]; + xi++; + zi++; + } + while (yi < ysize) { + ztable[zi] = y.table[yi]; + ztable[zi + zsize] = y.table[yi + ysize]; + yi++; + zi++; + } + + return new StarlarkInfoNoSchema(x.provider, ztable, Location.BUILTIN); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithMessage.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithMessage.java index c63e9c9a276385..cc0d477bab89ca 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithMessage.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithMessage.java @@ -20,7 +20,7 @@ import net.starlark.java.syntax.Location; /** - * A {@link StarlarkInfo} that supports custom no-such-field error messages. + * A struct-like object supporting a custom no-such-field error message. * *

This is used for certain special structs like `ctx.attr`. */ @@ -28,7 +28,7 @@ // class hierarchy of StructImpl at all. They really only need to have fields and custom error // messages (which are features of the simpler Structure class), not `+` concatenation, // proto/json encoding, or provider functionality. -public final class StarlarkInfoWithMessage extends StarlarkInfo { +public final class StarlarkInfoWithMessage extends StarlarkInfoNoSchema { // A format string with one %s placeholder for the missing field name. // TODO(adonovan): make the provider determine the error message // (but: this has implications for struct+struct, the equivalence @@ -38,8 +38,11 @@ public final class StarlarkInfoWithMessage extends StarlarkInfo { private final String unknownFieldError; private StarlarkInfoWithMessage( - Provider provider, Object[] table, @Nullable Location loc, String unknownFieldError) { - super(provider, table, loc); + Provider provider, + Map values, + @Nullable Location loc, + String unknownFieldError) { + super(provider, values, loc); this.unknownFieldError = unknownFieldError; } @@ -81,7 +84,6 @@ public String getErrorMessageForUnknownField(String name) { public static StarlarkInfo createWithCustomMessage( Provider provider, Map values, String unknownFieldError) { Preconditions.checkNotNull(unknownFieldError); - return new StarlarkInfoWithMessage( - provider, toTable(values), Location.BUILTIN, unknownFieldError); + return new StarlarkInfoWithMessage(provider, values, Location.BUILTIN, unknownFieldError); } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithSchema.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithSchema.java new file mode 100644 index 00000000000000..34dc41e307207c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkInfoWithSchema.java @@ -0,0 +1,156 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.packages; + +import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableCollection; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import javax.annotation.Nullable; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Starlark; +import net.starlark.java.syntax.Location; +import net.starlark.java.syntax.TokenKind; + +/** A struct-like Info (provider instance) for providers defined in Starlark that have a schema. */ +public class StarlarkInfoWithSchema extends StarlarkInfo { + private final StarlarkProvider provider; + + // For each field in provider.getFields the table contains on corresponding position either null + // or a legal Starlark value + private final Object[] table; + + private StarlarkInfoWithSchema( + StarlarkProvider provider, Object[] table, @Nullable Location loc) { + super(loc); + this.provider = provider; + this.table = table; + } + + @Override + public Provider getProvider() { + return provider; + } + + /** + * Constructs a StarlarkInfo from an array of alternating key/value pairs as provided by + * Starlark.fastcall. Checks that each key is provided at most once, and is defined by the schema, + * which must be sorted. This function exists solely for the StarlarkProvider constructor. + */ + static StarlarkInfoWithSchema createFromNamedArgs( + StarlarkProvider provider, Object[] table, Location loc) throws EvalException { + ImmutableList fields = provider.getFields(); + + Object[] valueTable = new Object[fields.size()]; + List unexpected = null; + + for (int i = 0; i < table.length; i += 2) { + int pos = Collections.binarySearch(fields, (String) table[i]); + if (pos >= 0) { + if (valueTable[pos] != null) { + throw Starlark.errorf( + "got multiple values for parameter %s in call to instantiate provider %s", + table[i], provider.getPrintableName()); + } + valueTable[pos] = table[i + 1]; + } else { + if (unexpected == null) { + unexpected = new ArrayList<>(); + } + unexpected.add((String) table[i]); + } + } + + if (unexpected != null) { + throw Starlark.errorf( + "got unexpected field%s '%s' in call to instantiate provider %s", + unexpected.size() > 1 ? "s" : "", + Joiner.on("', '").join(unexpected), + provider.getPrintableName()); + } + return new StarlarkInfoWithSchema(provider, valueTable, loc); + } + + @Override + public ImmutableCollection getFieldNames() { + ImmutableList.Builder fieldNames = new ImmutableList.Builder<>(); + ImmutableList fields = provider.getFields(); + for (int i = 0; i < fields.size(); i++) { + if (table[i] != null) { + fieldNames.add(fields.get(i)); + } + } + return fieldNames.build(); + } + + @Override + public boolean isImmutable() { + // If the provider is not yet exported, the hash code of the object is subject to change. + if (!provider.isExported()) { + return false; + } + for (int i = 0; i < table.length; i++) { + if (table[i] != null && !Starlark.isImmutable(table[i])) { + return false; + } + } + return true; + } + + @Nullable + @Override + public Object getValue(String name) { + ImmutableList fields = provider.getFields(); + int i = Collections.binarySearch(fields, name); + return i >= 0 ? table[i] : null; + } + + @Nullable + @Override + public StarlarkInfoWithSchema binaryOp(TokenKind op, Object that, boolean thisLeft) + throws EvalException { + if (op == TokenKind.PLUS && that instanceof StarlarkInfo) { + final Provider thatProvider = ((StarlarkInfo) that).getProvider(); + if (!provider.equals(thatProvider)) { + throw Starlark.errorf( + "Cannot use '+' operator on instances of different providers (%s and %s)", + provider.getPrintableName(), thatProvider.getPrintableName()); + } + Preconditions.checkArgument(that instanceof StarlarkInfoWithSchema); + return thisLeft + ? plus(this, (StarlarkInfoWithSchema) that) // + : plus((StarlarkInfoWithSchema) that, this); + } + return null; + } + + private static StarlarkInfoWithSchema plus(StarlarkInfoWithSchema x, StarlarkInfoWithSchema y) + throws EvalException { + int n = x.table.length; + + Object[] ztable = new Object[n]; + for (int i = 0; i < n; i++) { + if (x.table[i] != null && y.table[i] != null) { + ImmutableList schema = x.provider.getFields(); + throw Starlark.errorf("cannot add struct instances with common field '%s'", schema.get(i)); + } + ztable[i] = x.table[i] != null ? x.table[i] : y.table[i]; + } + return new StarlarkInfoWithSchema(x.provider, ztable, Location.BUILTIN); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java index 2b82f1acaec420..17e915b93de20d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java @@ -58,8 +58,7 @@ public final class StarlarkProvider implements StarlarkCallable, StarlarkExporta private final Location location; // For schemaful providers, the sorted list of allowed field names. - // The requirement for sortedness comes from StarlarkInfo.createFromNamedArgs, - // as it lets us verify table ⊆ schema in O(n) time without temporaries. + // The requirement for sortedness comes from StarlarkInfoWithSchema and lets us bisect the fields. @Nullable private final ImmutableList schema; // Optional custom initializer callback. If present, it is invoked with the same positional and @@ -180,11 +179,11 @@ public Object fastcall(StarlarkThread thread, Object[] positional, Object[] name } Object initResult = Starlark.fastcall(thread, init, positional, named); - return StarlarkInfo.createFromNamedArgs( - this, - toNamedArgs(initResult, "return value of provider init()"), - schema, - thread.getCallerLocation()); + // The code-path for providers with schema could be optimised to skip the call to toNamedArgs. + // As it is, we copy the map to an alternating key-value Object array, and then extract just + // the values into another array. + return createFromNamedArgs( + toNamedArgs(initResult, "return value of provider init()"), thread.getCallerLocation()); } private Object fastcallRawConstructor(StarlarkThread thread, Object[] positional, Object[] named) @@ -192,7 +191,13 @@ private Object fastcallRawConstructor(StarlarkThread thread, Object[] positional if (positional.length > 0) { throw Starlark.errorf("%s: unexpected positional arguments", getName()); } - return StarlarkInfo.createFromNamedArgs(this, named, schema, thread.getCallerLocation()); + return createFromNamedArgs(named, thread.getCallerLocation()); + } + + private StarlarkInfo createFromNamedArgs(Object[] named, Location loc) throws EvalException { + return schema != null + ? StarlarkInfoWithSchema.createFromNamedArgs(this, named, loc) + : StarlarkInfoNoSchema.createFromNamedArgs(this, named, loc); } private static final class RawConstructor implements StarlarkCallable { diff --git a/src/test/java/com/google/devtools/build/lib/packages/StarlarkInfoTest.java b/src/test/java/com/google/devtools/build/lib/packages/StarlarkInfoTest.java index 926384c6a5fb06..e3fb773b35f9bf 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/StarlarkInfoTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/StarlarkInfoTest.java @@ -113,7 +113,7 @@ public void concatWithSameFields() throws Exception { StarlarkProvider provider = makeProvider(); StarlarkInfo info1 = makeInfoWithF1F2Values(provider, StarlarkInt.of(4), null); StarlarkInfo info2 = makeInfoWithF1F2Values(provider, null, StarlarkInt.of(5)); - StarlarkInfo result = info1.binaryOp(TokenKind.PLUS, info2, true); + StarlarkInfo result = (StarlarkInfo) info1.binaryOp(TokenKind.PLUS, info2, true); assertThat(result.getFieldNames()).containsExactly("f1", "f2"); assertThat(result.getValue("f1")).isEqualTo(StarlarkInt.of(4)); assertThat(result.getValue("f2")).isEqualTo(StarlarkInt.of(5)); @@ -124,7 +124,7 @@ public void concatWithDifferentFields() throws Exception { StarlarkProvider provider = makeProvider(); StarlarkInfo info1 = makeInfoWithF1F2Values(provider, StarlarkInt.of(4), null); StarlarkInfo info2 = makeInfoWithF1F2Values(provider, null, StarlarkInt.of(5)); - StarlarkInfo result = info1.binaryOp(TokenKind.PLUS, info2, true); + StarlarkInfo result = (StarlarkInfo) info1.binaryOp(TokenKind.PLUS, info2, true); assertThat(result.getFieldNames()).containsExactly("f1", "f2"); assertThat(result.getValue("f1")).isEqualTo(StarlarkInt.of(4)); assertThat(result.getValue("f2")).isEqualTo(StarlarkInt.of(5)); @@ -170,7 +170,7 @@ public void testPermute() throws Exception { array[2 * i] = i + 1; // keys are positive array[2 * i + 1] = -i - 1; // value is negation of corresponding key } - StarlarkInfo.permute(array); + StarlarkInfoNoSchema.permute(array); // Assert that keys (positive) appear before values (negative). for (int i = 0; i < 2 * a; i++) { @@ -246,7 +246,7 @@ public void testSortPairs() throws Exception { // Sort using sortPairs. if (a > 0) { - StarlarkInfo.sortPairs(array, 0, a - 1); + StarlarkInfoNoSchema.sortPairs(array, 0, a - 1); } // Assert sorted keys match reference implementation.