Repository: bazelbuild/bazel
Issue: Analysis performance regression when lockfile is enabled
Labels: type: bug
P1
team-ExternalDeps
area-Bzlmod

### Description of the bug:


While migrating our repo to bzlmod we encountered an 3x slowdown in analysis when using bzlmod.

Looking further into it it seems like it's caused by the newly introduced lockfile because the performance returns back to normal when `--lockfile_mode=off` is set.

I can unfortunately not share an reproduction since the repo is private. The repo is using MODULE.bazel and WORKSPACE.bzlmod since not all of our external deps have been migrated to bzlmod.

I'll share a profile with and without the lockfile:
[timing_profile_with_lockfile.gz](https://github.com/bazelbuild/bazel/files/13175335/timing_profile_with_lockfile.gz)
[timing_profile_without_lockfile.gz](https://github.com/bazelbuild/bazel/files/13175338/timing_profile_without_lockfile.gz)

Tagging @fmeum and @Wyverald since I had talked to them on Slack 

### Which operating system are you running Bazel on?


Linux Fedora 38

### What is the output of `bazel info release`?


release 7.0.0rc2



Issue comments:
@SalmaSamy @meteorcloudy 
I'm going to preemptively mark this as a release blocker. 3x slowdown is unacceptable
@bazel-io fork 7.0.0
@purkhusid Could you share the output of `bazel info` after each of these builds?
With `lockfile_mode=off` (25s runtime):
```
bazel-bin: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/bin
bazel-genfiles: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/bin
bazel-testlogs: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/testlogs
character-encoding: file.encoding = ISO-8859-1, defaultCharset = ISO-8859-1
command_log: /workspaces/bazel_output_base/command.log
committed-heap-size: 6954MB
execution_root: /workspaces/bazel_output_base/execroot/_main
gc-count: 59
gc-time: 1137ms
install_base: /workspaces/bazel_user_root/install/e0df88ff6ee693b65a8a08e8e987fb40
java-home: /workspaces/bazel_user_root/install/e0df88ff6ee693b65a8a08e8e987fb40/embedded_tools/jdk
java-runtime: OpenJDK Runtime Environment (build 21+35) by Azul Systems, Inc.
java-vm: OpenJDK 64-Bit Server VM (build 21+35, mixed mode) by Azul Systems, Inc.
local_resources: RAM=64290MB, CPU=16.0
max-heap-size: 16861MB
output_base: /workspaces/bazel_output_base
output_path: /workspaces/bazel_output_base/execroot/_main/bazel-out
package_path: %workspace%
release: release 7.0.0rc2
repository_cache: null
server_log: /workspaces/bazel_output_base/java.log.codespaces-4c3412.vscode.log.java.20231026-201446.5072
server_pid: 5072
used-heap-size: 2798MB
workspace: /workspaces/monorepo
```

With `--lockfile_mode=update` (96s runtime):
```
bazel-bin: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/bin
bazel-genfiles: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/bin
bazel-testlogs: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/testlogs
character-encoding: file.encoding = ISO-8859-1, defaultCharset = ISO-8859-1
command_log: /workspaces/bazel_output_base/command.log
committed-heap-size: 4445MB
execution_root: /workspaces/bazel_output_base/execroot/_main
gc-count: 143
gc-time: 2837ms
install_base: /workspaces/bazel_user_root/install/e0df88ff6ee693b65a8a08e8e987fb40
java-home: /workspaces/bazel_user_root/install/e0df88ff6ee693b65a8a08e8e987fb40/embedded_tools/jdk
java-runtime: OpenJDK Runtime Environment (build 21+35) by Azul Systems, Inc.
java-vm: OpenJDK 64-Bit Server VM (build 21+35, mixed mode) by Azul Systems, Inc.
local_resources: RAM=64290MB, CPU=16.0
max-heap-size: 16861MB
output_base: /workspaces/bazel_output_base
output_path: /workspaces/bazel_output_base/execroot/_main/bazel-out
package_path: %workspace%
release: release 7.0.0rc2
repository_cache: null
server_log: /workspaces/bazel_output_base/java.log.codespaces-4c3412.vscode.log.java.20231026-201700.7299
server_pid: 7299
used-heap-size: 3670MB
workspace: /workspaces/monorepo
```

With `--lockfile_mode=error` (96s runtime):
```
bazel-bin: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/bin
bazel-genfiles: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/bin
bazel-testlogs: /workspaces/bazel_output_base/execroot/_main/bazel-out/k8-fastbuild/testlogs
character-encoding: file.encoding = ISO-8859-1, defaultCharset = ISO-8859-1
command_log: /workspaces/bazel_output_base/command.log
committed-heap-size: 3019MB
execution_root: /workspaces/bazel_output_base/execroot/_main
gc-count: 139
gc-time: 3255ms
install_base: /workspaces/bazel_user_root/install/e0df88ff6ee693b65a8a08e8e987fb40
java-home: /workspaces/bazel_user_root/install/e0df88ff6ee693b65a8a08e8e987fb40/embedded_tools/jdk
java-runtime: OpenJDK Runtime Environment (build 21+35) by Azul Systems, Inc.
java-vm: OpenJDK 64-Bit Server VM (build 21+35, mixed mode) by Azul Systems, Inc.
local_resources: RAM=64290MB, CPU=16.0
max-heap-size: 16861MB
output_base: /workspaces/bazel_output_base
output_path: /workspaces/bazel_output_base/execroot/_main/bazel-out
package_path: %workspace%
release: release 7.0.0rc2
repository_cache: null
server_log: /workspaces/bazel_output_base/java.log.codespaces-4c3412.vscode.log.java.20231026-201940.9941
server_pid: 9941
used-heap-size: 1371MB
workspace: /workspaces/monorepo
```

I did `bazel shutdown` between each invocation.
I am able to reproduce the issue with a synthetic module extension that generates 2000 repos. This is the call stack responsible for the slowdown:
![IMG-20231028-WA0001.jpg](https://github.com/bazelbuild/bazel/assets/4312191/5c4b293a-7529-4880-a90e-530c56671af9)


@purkhusid Are you able to run Bazel built from source (`bazel build //src:bazel`)? If so, could you build and test https://github.com/bazelbuild/bazel/pull/19970?
Ran the proposed fix on on our repo:
With `lockfile_mode=off`: 50s
With `lockfile_mode=update`: 41s

Note that it's not the same machine as I used for the earlier runs so the overall slowdown compared to that is expected

The proposed fix seems to have worked and then some additional perf gain!

A fix for this issue has been included in [Bazel 7.0.0 RC5](https://releases.bazel.build/7.0.0/rc5/index.html). Please test out the release candidate and report any issues as soon as possible. Thanks!

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java
index b2e0f6f3a5b40f..cd9c1e6d5bcddf 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java
@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
-import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableTable;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
 
@@ -22,20 +21,39 @@
  * After resolving bazel module this event is sent from {@link BazelDepGraphFunction} holding the
  * lockfile value with module updates, and the module extension usages. It will be received in
  * {@link BazelLockFileModule} to be used to update the lockfile content
+ *
+ * <p>Instances of this class are retained in Skyframe nodes and subject to frequent {@link
+ * java.util.Set}-based deduplication. As such, it <b>must</b> have a cheap implementation of {@link
+ * #hashCode()} and {@link #equals(Object)}. It currently uses reference equality since the logic
+ * that creates instances of this class already ensures that there is only one instance per build.
  */
-@AutoValue
-public abstract class BazelModuleResolutionEvent implements Postable {
+public final class BazelModuleResolutionEvent implements Postable {
+
+  private final BazelLockFileValue lockfileValue;
+  private final ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
+      extensionUsagesById;
+
+  private BazelModuleResolutionEvent(
+      BazelLockFileValue lockfileValue,
+      ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> extensionUsagesById) {
+    this.lockfileValue = lockfileValue;
+    this.extensionUsagesById = extensionUsagesById;
+  }
 
   public static BazelModuleResolutionEvent create(
       BazelLockFileValue lockFileValue,
       ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> extensionUsagesById) {
-    return new AutoValue_BazelModuleResolutionEvent(lockFileValue, extensionUsagesById);
+    return new BazelModuleResolutionEvent(lockFileValue, extensionUsagesById);
   }
 
-  public abstract BazelLockFileValue getLockfileValue();
+  public BazelLockFileValue getLockfileValue() {
+    return lockfileValue;
+  }
 
-  public abstract ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
-      getExtensionUsagesById();
+  public ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
+      getExtensionUsagesById() {
+    return extensionUsagesById;
+  }
 
   @Override
   public boolean storeForReplay() {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionEvent.java
index 6b93cc4e77332a..893608bef70afe 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionEvent.java
@@ -14,30 +14,53 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
-import com.google.auto.value.AutoValue;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
 
 /**
  * After evaluating any module extension this event is sent from {@link SingleExtensionEvalFunction}
  * holding the extension id and the resolution data LockFileModuleExtension. It will be received in
- * {@link BazelLockFileModule} to be used to update the lockfile content
+ * {@link BazelLockFileModule} to be used to update the lockfile content.
+ *
+ * <p>Instances of this class are retained in Skyframe nodes and subject to frequent {@link
+ * java.util.Set}-based deduplication. As such, it <b>must</b> have a cheap implementation of {@link
+ * #hashCode()} and {@link #equals(Object)}. It currently uses reference equality since the logic
+ * that creates instances of this class already ensures that there is only one instance per
+ * extension id.
  */
-@AutoValue
-public abstract class ModuleExtensionResolutionEvent implements Postable {
+public final class ModuleExtensionResolutionEvent implements Postable {
+
+  private final ModuleExtensionId extensionId;
+  private final ModuleExtensionEvalFactors extensionFactors;
+  private final LockFileModuleExtension moduleExtension;
+
+  private ModuleExtensionResolutionEvent(
+      ModuleExtensionId extensionId,
+      ModuleExtensionEvalFactors extensionFactors,
+      LockFileModuleExtension moduleExtension) {
+    this.extensionId = extensionId;
+    this.extensionFactors = extensionFactors;
+    this.moduleExtension = moduleExtension;
+  }
 
   public static ModuleExtensionResolutionEvent create(
       ModuleExtensionId extensionId,
       ModuleExtensionEvalFactors extensionFactors,
       LockFileModuleExtension lockfileModuleExtension) {
-    return new AutoValue_ModuleExtensionResolutionEvent(
+    return new ModuleExtensionResolutionEvent(
         extensionId, extensionFactors, lockfileModuleExtension);
   }
 
-  public abstract ModuleExtensionId getExtensionId();
+  public ModuleExtensionId getExtensionId() {
+    return extensionId;
+  }
 
-  public abstract ModuleExtensionEvalFactors getExtensionFactors();
+  public ModuleExtensionEvalFactors getExtensionFactors() {
+    return extensionFactors;
+  }
 
-  public abstract LockFileModuleExtension getModuleExtension();
+  public LockFileModuleExtension getModuleExtension() {
+    return moduleExtension;
+  }
 
   @Override
   public boolean storeForReplay() {
diff --git a/src/main/java/com/google/devtools/build/lib/events/Reportable.java b/src/main/java/com/google/devtools/build/lib/events/Reportable.java
index 4601fc4ef172c1..cf8f757c5a665c 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Reportable.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Reportable.java
@@ -47,6 +47,9 @@ public interface Reportable {
    *
    * <p>This method is not relevant for events which do not originate from {@link
    * com.google.devtools.build.skyframe.SkyFunction} evaluation.
+   *
+   * <p>Classes returning {@code true} should have cheap {@link Object#hashCode()} and {@link
+   * Object#equals(Object)} implementations.
    */
   default boolean storeForReplay() {
     return false;


Test patch: