Sort out of order transitive packages before adding. (#18666)

PiperOrigin-RevId: 541656611
Change-Id: Ie6c365275003343dc2bf0520a305e005ec48550b

Author: aoeui

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -2361,19 +2361,19 @@ java_library(
     srcs = ["constraints/IncompatibleTargetChecker.java"],
     deps = [
         ":analysis_cluster",
+        ":config/build_configuration",
+        ":config/config_conditions",
+        ":configured_target",
+        ":configured_target_value",
         ":constraints/environment_collection",
         ":constraints/supported_environments",
+        ":dependency_kind",
         ":file_provider",
         ":incompatible_platform_provider",
+        ":target_and_configuration",
         ":test/test_configuration",
+        ":transitive_dependency_state",
         ":transitive_info_provider_map_builder",
-        "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
-        "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
-        "//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions",
-        "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
-        "//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
-        "//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind",
-        "//src/main/java/com/google/devtools/build/lib/analysis:target_and_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
@@ -2782,3 +2782,20 @@ java_library(
         "//third_party:guava",
     ],
 )
+
+java_library(
+    name = "transitive_dependency_state",
+    srcs = ["TransitiveDependencyState.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/causes",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+        "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)

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

@@ -0,0 +1,273 @@
+// 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.analysis;
+
+import static com.google.common.collect.Comparators.lexicographical;
+import static java.util.Comparator.comparing;
+import static java.util.Comparator.naturalOrder;
+import static java.util.Comparator.nullsFirst;
+
+import com.google.devtools.build.lib.causes.Cause;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.packages.AspectDescriptor;
+import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.TreeMap;
+import java.util.concurrent.ConcurrentHashMap;
+import javax.annotation.Nullable;
+
+/** Groups state associated with transitive dependencies. */
+public final class TransitiveDependencyState {
+  private final NestedSetBuilder<Cause> transitiveRootCauses;
+
+  /**
+   * State for constructing the packages transitively loaded for the value being built.
+   *
+   * <p>See {@link
+   * com.google.devtools.build.lib.analysis.ConfiguredObjectValue#getTransitivePackages}.
+   *
+   * <p>Non-null when transitive packages are tracked, determined by {@link
+   * com.google.devtools.build.lib.skyframe.SkyframeExecutor#shouldStoreTransitivePackagesInLoadingAndAnalysis}.
+   */
+  @Nullable private final PackageCollector packageCollector;
+
+  /**
+   * Contains packages that were previously requested by transitive dependencies.
+   *
+   * <p>When the {@link ConfiguredTargetFunction} computes a value, it depends on properties of its
+   * dependencies. In some cases, those values are read directly out of the dependency's underlying
+   * {@link Target}. All instances of this are to be restricted to where {@link
+   * ConfiguredTargetAndData#target} is read.
+   *
+   * <p>More ideally, those properties would be conveyed via providers of those dependencies, but
+   * doing so would adversely affect resting heap usage whereas {@link ConfiguredTargetAndData} is
+   * ephemeral. Distributed implementations will include these properties in an extra providers. It
+   * won't affect memory because the underlying package won't exist on the node loading it remotely.
+   *
+   * <p>It's valid to obtain {@link Package}s of dependencies from this map instead of creating an
+   * edge in {@code Skyframe} due to the transitive dependency through the {@link ConfiguredTarget}
+   * if it is scoped to the invocation. Invalidation of the {@link Package} propagates upwards
+   * through the dependency. This is compatible with bottom-up change pruning because {@link
+   * ConfiguredTargetValue} uses identity equals.
+   */
+  @Nullable // TODO(b/261521010): make this non-null.
+  private final ConcurrentHashMap<PackageIdentifier, Package> prerequisitePackages;
+
+  public TransitiveDependencyState(
+      boolean storeTransitivePackages,
+      @Nullable ConcurrentHashMap<PackageIdentifier, Package> prerequisitePackages) {
+    this.transitiveRootCauses = NestedSetBuilder.stableOrder();
+    this.packageCollector = storeTransitivePackages ? new PackageCollector() : null;
+    this.prerequisitePackages = prerequisitePackages;
+  }
+
+  public static TransitiveDependencyState createForTesting() {
+    return new TransitiveDependencyState(
+        /* storeTransitivePackages= */ false,
+        // Passing an empty map here means that there will be few, if any, existing prerequisite
+        // Packages. This causes the underlying code to fall back on declaring Package edges for
+        // prerequisites.
+        new ConcurrentHashMap<>());
+  }
+
+  public NestedSetBuilder<Cause> transitiveRootCauses() {
+    return transitiveRootCauses;
+  }
+
+  @Nullable
+  public NestedSet<Package> transitivePackages() {
+    if (packageCollector == null) {
+      return null;
+    }
+    return packageCollector.buildSet();
+  }
+
+  public void addTransitiveCauses(NestedSet<Cause> transitiveCauses) {
+    transitiveRootCauses.addTransitive(transitiveCauses);
+  }
+
+  public void addTransitiveCause(Cause cause) {
+    transitiveRootCauses.add(cause);
+  }
+
+  public boolean storeTransitivePackages() {
+    return packageCollector != null;
+  }
+
+  /** Adds to the set of transitive packages if {@link #storeTransitivePackages} is true. */
+  public void updateTransitivePackages(Package pkg) {
+    if (packageCollector == null) {
+      return;
+    }
+    packageCollector.packages.add(pkg);
+  }
+
+  /** Adds to the set of transitive packages if {@link #storeTransitivePackages} is true. */
+  public void updateTransitivePackages(ConfiguredTargetKey key, NestedSet<Package> packages) {
+    if (packageCollector == null) {
+      return;
+    }
+    packageCollector.configuredTargetPackages.put(key, packages);
+  }
+
+  /** Adds to the set of transitive packages if {@link #storeTransitivePackages} is true. */
+  public void updateTransitivePackages(AspectKey key, NestedSet<Package> packages) {
+    if (packageCollector == null) {
+      return;
+    }
+    packageCollector.aspectPackages.put(key, packages);
+  }
+
+  /** Sets a batch of transitive packages to be added. */
+  public void setTransitivePackagesBatch(NestedSet<Package> packages) {
+    if (packageCollector == null) {
+      return;
+    }
+    packageCollector.transitivePackagesBatch = packages;
+  }
+
+  @Nullable
+  public Package getDependencyPackage(PackageIdentifier packageId) {
+    if (prerequisitePackages == null) {
+      return null;
+    }
+    return prerequisitePackages.get(packageId);
+  }
+
+  public void putDependencyPackageIfAbsent(PackageIdentifier packageId, Package pkg) {
+    if (prerequisitePackages == null) {
+      return;
+    }
+    prerequisitePackages.putIfAbsent(packageId, pkg);
+  }
+
+  /**
+   * Collects packages of dependencies to be unified in a {@link NestedSet}.
+   *
+   * <p>Performs bookkeeping so the result is deterministic.
+   *
+   * <p>Work in Skyframe may complete in arbitrary order due to missing values and restarts. For
+   * example, if a client requests {@code //foo} and {@code //bar}, it could receive any of the
+   * following: {@code (//foo, null), (null, //bar), (//foo, //bar) or (null, null)}.
+   *
+   * <p>This class tracks how the {@link Package}s are added so they can be given a deterministic
+   * order. This is required for determinism of {@link
+   * com.google.devtools.build.lib.analysis.RepoMappingManifestAction#computeKey}.
+   */
+  private static class PackageCollector {
+    /**
+     * Keeps packages that were added directly as a list.
+     *
+     * <p>These will be sorted.
+     */
+    private final ArrayList<Package> packages = new ArrayList<>();
+
+    /** Stores transitive {@link Package}s of {@link ConfiguredTargetValues}s. */
+    private final TreeMap<ConfiguredTargetKey, NestedSet<Package>> configuredTargetPackages =
+        new TreeMap<>(CONFIGURED_TARGET_KEY_ORDERING);
+
+    /** Stores transitive {@link Package}s of {@link AspectValue}s. */
+    private final TreeMap<AspectKey, NestedSet<Package>> aspectPackages =
+        new TreeMap<>(ASPECT_KEY_ORDERING);
+
+    /**
+     * Stores a single batch of packages computed by {@link
+     * PrerequisiteProducer#resolveConfiguredTargetDependencies}.
+     */
+    // TODO(b/261521010): delete this when the corresponding method is deleted.
+    private NestedSet<Package> transitivePackagesBatch;
+
+    /**
+     * Constructs the deterministically ordered result.
+     *
+     * <p>It's safe to call this multiple times.
+     */
+    private NestedSet<Package> buildSet() {
+      var result = NestedSetBuilder.<Package>stableOrder();
+
+      Collections.sort(packages, comparing(Package::getPackageIdentifier));
+      result.addAll(packages);
+
+      for (NestedSet<Package> packageSet : configuredTargetPackages.values()) {
+        result.addTransitive(packageSet);
+      }
+      for (NestedSet<Package> packageSet : aspectPackages.values()) {
+        result.addTransitive(packageSet);
+      }
+
+      if (transitivePackagesBatch != null) {
+        result.addTransitive(transitivePackagesBatch);
+      }
+
+      return result.build();
+    }
+  }
+
+  private static final Comparator<ConfiguredTargetKey> CONFIGURED_TARGET_KEY_ORDERING =
+      comparing(ConfiguredTargetKey::getLabel)
+          .thenComparing(ConfiguredTargetKey::getExecutionPlatformLabel, nullsFirst(naturalOrder()))
+          .thenComparing(
+              ConfiguredTargetKey::getConfigurationKey,
+              nullsFirst(comparing(BuildConfigurationKey::getOptionsChecksum)));
+
+  private static final Comparator<AspectKey> ASPECT_KEY_ORDERING =
+      comparing(AspectKey::getBaseConfiguredTargetKey, CONFIGURED_TARGET_KEY_ORDERING)
+          .thenComparing(
+              (left, right) -> new AspectKeyDescriptorGraphComparator().compare(left, right));
+
+  /**
+   * Compares the {@link AspectKey} graph structure for specific dependencies.
+   *
+   * <p>An {@link AspectKey} for a dependency is determined by {@link
+   * AspectCollection#buildAspectKey}. This means that the {@link AspectKey} is structured like a
+   * DAG with the following properties.
+   *
+   * <ul>
+   *   <li>The {@link AspectKey#getBaseConfiguredTargetKey} is the same across all nodes.
+   *   <li>Each DAG node has a unique {@link AspectKey#getAspectDescriptor}.
+   * </ul>
+   *
+   * <p>Given the above, it's sufficient to traverse unique {@link AspectDescriptor}s to understand
+   * the toplogy of both graphs.
+   *
+   * <p>NB: a new instance of this comparator must be constructed for each comparison.
+   */
+  private static class AspectKeyDescriptorGraphComparator implements Comparator<AspectKey> {
+    private final HashSet<AspectDescriptor> visited = new HashSet<>();
+
+    @Override
+    public int compare(AspectKey left, AspectKey right) {
+      AspectDescriptor leftDescriptor = left.getAspectDescriptor();
+      AspectDescriptor rightDescriptor = right.getAspectDescriptor();
+      if (!leftDescriptor.equals(rightDescriptor)) {
+        return leftDescriptor.getDescription().compareTo(rightDescriptor.getDescription());
+      }
+      if (!visited.add(leftDescriptor)) {
+        return 0;
+      }
+
+      return lexicographical(this).compare(left.getBaseKeys(), right.getBaseKeys());
+    }
+  }
+}