Cache allowlists and decisions

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java

@@ -42,7 +42,6 @@
 import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams;
 import com.google.devtools.build.lib.analysis.test.TestTagsProvider;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -371,8 +370,7 @@ private void propagateTransitiveValidationOutputGroups() {
       // only allow native and builtins to override transitive validation propagation
       if (rdeLabel != null
           && BuiltinRestriction.isNotAllowed(
-              rdeLabel,
-              RepositoryMapping.EMPTY,
+              rdeLabel.getPackageIdentifier(),
               BuiltinRestriction.INTERNAL_STARLARK_API_ALLOWLIST)) {
         ruleContext.ruleError(rdeLabel + " cannot access the _transitive_validation private API");
         return;

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -1836,9 +1836,7 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
               // allow setting private attributes from initializers in builtins
               Label definitionLabel = currentRuleClass.getRuleDefinitionEnvironmentLabel();
               BuiltinRestriction.failIfLabelOutsideAllowlist(
-                  definitionLabel,
-                  targetDefinitionContext.getMainRepoMapping(),
-                  ALLOWLIST_RULE_EXTENSION_API_EXPERIMENTAL);
+                  definitionLabel, ALLOWLIST_RULE_EXTENSION_API_EXPERIMENTAL);
             }
             String nativeName = arg.startsWith("_") ? "$" + arg.substring(1) : arg;
             Attribute attr =

src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java

@@ -13,11 +13,14 @@
 // limitations under the License.
 package com.google.devtools.build.lib.packages;
 
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.LoadingCache;
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.RepositoryMapping;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 import net.starlark.java.eval.EvalException;
@@ -47,15 +50,13 @@ public final class BuiltinRestriction {
               BuiltinRestriction.allowlistEntry("", "tools/build_defs/android"),
               BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_android"),
               BuiltinRestriction.allowlistEntry("rules_android", ""),
-              BuiltinRestriction.allowlistEntry("build_bazel_rules_android", ""),
 
               // Apple rules
               BuiltinRestriction.allowlistEntry("", "third_party/apple_crosstool"),
               BuiltinRestriction.allowlistEntry(
                   "", "third_party/cpptoolchains/portable_llvm/build_defs"),
               BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_apple"),
               BuiltinRestriction.allowlistEntry("rules_apple", ""),
-              BuiltinRestriction.allowlistEntry("build_bazel_rules_apple", ""),
 
               // Cc rules
               BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_cc"),
@@ -84,7 +85,6 @@ public final class BuiltinRestriction {
               // Proto rules
               BuiltinRestriction.allowlistEntry("", "third_party/protobuf"),
               BuiltinRestriction.allowlistEntry("protobuf", ""),
-              BuiltinRestriction.allowlistEntry("com_google_protobuf", ""),
 
               // Shell rules
               BuiltinRestriction.allowlistEntry("rules_shell", ""));
@@ -120,9 +120,24 @@ static AllowlistEntry create(String apparentRepoName, PathFragment packagePrefix
       return new AutoValue_BuiltinRestriction_AllowlistEntry(apparentRepoName, packagePrefix);
     }
 
-    final boolean allows(Label label, RepositoryMapping repoMapping) {
-      return label.getRepository().equals(repoMapping.get(apparentRepoName()))
-          && label.getPackageFragment().startsWith(packagePrefix());
+    final boolean allows(PackageIdentifier packageIdentifier) {
+      return reposMatch(apparentRepoName(), packageIdentifier.getRepository())
+          && packageIdentifier.getPackageFragment().startsWith(packagePrefix());
+    }
+
+    private static boolean reposMatch(String allowedName, RepositoryName givenName) {
+      if (allowedName.equals(RepositoryName.MAIN.getName())) {
+        return givenName.equals(RepositoryName.MAIN);
+      }
+      if (allowedName.equals(RepositoryName.BAZEL_TOOLS.getName())) {
+        return givenName.equals(RepositoryName.BAZEL_TOOLS);
+      }
+      if (allowedName.equals(RepositoryName.BUILTINS.getName())) {
+        return givenName.equals(RepositoryName.BUILTINS);
+      }
+      // allowedName is a module name and givenName is a real canonical repo name, so it belongs to
+      // any version of that module if and only if it contains <allowedName>+ as a prefix.
+      return givenName.getName().startsWith(allowedName + "+");
     }
   }
 
@@ -165,30 +180,41 @@ public static void failIfCalledOutsideDefaultAllowlist(StarlarkThread thread)
    */
   public static void failIfModuleOutsideAllowlist(
       BazelModuleContext moduleContext, Collection<AllowlistEntry> allowlist) throws EvalException {
-    failIfLabelOutsideAllowlist(moduleContext.label(), moduleContext.repoMapping(), allowlist);
+    failIfLabelOutsideAllowlist(moduleContext.label(), allowlist);
   }
 
   /**
    * Throws {@code EvalException} if the given {@link Label} is not within either 1) the builtins
    * repository, or 2) a package or subpackage of an entry in the given allowlist.
    */
-  public static void failIfLabelOutsideAllowlist(
-      Label label, RepositoryMapping repoMapping, Collection<AllowlistEntry> allowlist)
+  public static void failIfLabelOutsideAllowlist(Label label, Collection<AllowlistEntry> allowlist)
       throws EvalException {
-    if (isNotAllowed(label, repoMapping, allowlist)) {
+    if (isNotAllowed(label.getPackageIdentifier(), allowlist)) {
       throw Starlark.errorf("file '%s' cannot use private API", label.getCanonicalForm());
     }
   }
 
+  private static final LoadingCache<
+          Collection<AllowlistEntry>, LoadingCache<PackageIdentifier, Boolean>>
+      IS_NOT_ALLOWED_CACHE =
+          Caffeine.newBuilder()
+              .initialCapacity(5)
+              .weakKeys()
+              .build(
+                  allowlist ->
+                      Caffeine.newBuilder()
+                          .weakKeys()
+                          .build(pkgId -> allowlist.stream().noneMatch(e -> e.allows(pkgId))));
+
   /**
-   * Returns true if the given {@link Label} is not within both 1) the builtins repository, or 2) a
-   * package or subpackage of an entry in the given allowlist.
+   * Returns true if the given {@link PackageIdentifier} is not within both 1) the builtins
+   * repository, or 2) a package or subpackage of an entry in the given allowlist.
    */
   public static boolean isNotAllowed(
-      Label label, RepositoryMapping repoMapping, Collection<AllowlistEntry> allowlist) {
-    if (label.getRepository().getName().equals("_builtins")) {
+      PackageIdentifier packageIdentifier, Collection<AllowlistEntry> allowlist) {
+    if (packageIdentifier.getRepository().getName().equals("_builtins")) {
       return false;
     }
-    return allowlist.stream().noneMatch(e -> e.allows(label, repoMapping));
+    return IS_NOT_ALLOWED_CACHE.get(allowlist).get(packageIdentifier);
   }
 }

src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java

@@ -17,7 +17,10 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.devtools.build.lib.rules.cpp.CcModule.nullIfNone;
 
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.LoadingCache;
 import com.google.common.base.Strings;
+import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -63,6 +66,7 @@
 import com.google.devtools.build.lib.starlarkbuildapi.NativeComputedDefaultApi;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Objects;
+import java.util.concurrent.CompletionException;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 import javax.annotation.Nullable;
@@ -89,6 +93,24 @@ public class CcStarlarkInternal implements StarlarkValue {
 
   public static final String NAME = "cc_internal";
 
+  // Avoid repeated parsing of allowlists, which previously accounted a non-negligible percentage
+  // of Bazel analysis CPU time (~1%). Also enables caching by identity in BuiltinRestriction.
+  private static final LoadingCache<Object, ImmutableList<BuiltinRestriction.AllowlistEntry>>
+      ALLOWLIST_CACHE =
+          Caffeine.newBuilder()
+              .initialCapacity(5)
+              .weakKeys()
+              .build(
+                  allowlistObject ->
+                      Sequence.cast(allowlistObject, Tuple.class, "allowlist").stream()
+                          // TODO(bazel-team): Avoid unchecked indexing and casts on values
+                          // obtained from Starlark, even though it is allowlisted.
+                          .map(
+                              p ->
+                                  BuiltinRestriction.allowlistEntry(
+                                      (String) p.get(0), (String) p.get(1)))
+                          .collect(toImmutableList()));
+
   @StarlarkMethod(
       name = "check_private_api",
       documented = false,
@@ -122,13 +144,14 @@ public void checkPrivateApi(Object allowlistObject, Object depth, StarlarkThread
       return;
     }
     BazelModuleContext bazelModuleContext = (BazelModuleContext) module.getClientData();
-    ImmutableList<BuiltinRestriction.AllowlistEntry> allowlist =
-        Sequence.cast(allowlistObject, Tuple.class, "allowlist").stream()
-            // TODO(bazel-team): Avoid unchecked indexing and casts on values obtained from
-            // Starlark, even though it is allowlisted.
-            .map(p -> BuiltinRestriction.allowlistEntry((String) p.get(0), (String) p.get(1)))
-            .collect(toImmutableList());
-    BuiltinRestriction.failIfModuleOutsideAllowlist(bazelModuleContext, allowlist);
+    try {
+      BuiltinRestriction.failIfModuleOutsideAllowlist(
+          bazelModuleContext, ALLOWLIST_CACHE.get(allowlistObject));
+    } catch (CompletionException e) {
+      // Thrown by ALLOWLIST_CACHE.
+      Throwables.throwIfInstanceOf(e.getCause(), EvalException.class);
+      throw new IllegalStateException("Unexpected exception type", e);
+    }
   }
 
   /** Wraps a dictionary of build variables into CcToolchainVariables. */

src/main/starlark/builtins_bzl/common/cc/cc_common_bazel.bzl

@@ -62,11 +62,13 @@ def _implementation_deps_allowed_by_allowlist(*, ctx):
     _cc_internal.check_private_api(allowlist = _PRIVATE_STARLARKIFICATION_ALLOWLIST)
     return _cc_common_internal.implementation_deps_allowed_by_allowlist(ctx = ctx)
 
+_INTERNAL_EXPORTS_ALLOWLIST = [
+    ("", "third_party/bazel_rules/rules_cc"),
+    ("rules_cc", ""),
+]
+
 def _internal_exports():
-    _cc_internal.check_private_api(allowlist = [
-        ("", "third_party/bazel_rules/rules_cc"),
-        ("rules_cc", ""),
-    ])
+    _cc_internal.check_private_api(allowlist = _INTERNAL_EXPORTS_ALLOWLIST)
     return _cc_internal
 
 cc_common = struct(

src/main/starlark/builtins_bzl/common/java/java_common.bzl

@@ -16,12 +16,14 @@
 
 _java_common_internal = _builtins.internal.java_common_internal_do_not_use
 
+_INTERNAL_EXPORTS_ALLOWLIST = [
+    ("", "javatests/com/google/devtools/grok/kythe/analyzers/build/testdata/pkg"),
+    ("", "third_party/bazel_rules/rules_java"),
+    ("rules_java", ""),
+]
+
 def _internal_exports():
-    _builtins.internal.cc_internal.check_private_api(allowlist = [
-        ("", "javatests/com/google/devtools/grok/kythe/analyzers/build/testdata/pkg"),
-        ("", "third_party/bazel_rules/rules_java"),
-        ("rules_java", ""),
-    ])
+    _builtins.internal.cc_internal.check_private_api(allowlist = _INTERNAL_EXPORTS_ALLOWLIST)
     return struct(
         create_compilation_action = _java_common_internal.create_compilation_action,
         create_header_compilation_action = _java_common_internal.create_header_compilation_action,