From 89778912d0eb038cbe0e7d9978103b9f7772b899 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 24 Sep 2024 14:36:45 -0700 Subject: [PATCH] Don't zero-out builder fields in finishBuild() The builder itself shouldn't be retained after package construction, so there shouldn't be a need to nullify its fields. Running the benchmark script shows this CL has no effect on memory. The line wrapping `targets` in an unmodifiable map dates back to unknown commit, and at that point was needed because Package didn't copy the map itself in `finishInit()` but just pointed to the builder's map. Eliminating these nullifications makes it easier to make these fields `private` in `TargetRegistrationEnvironment`. Work toward #19922. PiperOrigin-RevId: 678401129 Change-Id: Ica887b69fb795aaebf00d86e666b2817aa0ef659 --- .../com/google/devtools/build/lib/packages/Package.java | 6 +----- .../build/lib/packages/TargetRegistrationEnvironment.java | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index 473153c00c1544..c33f4331cef431 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -2092,14 +2092,10 @@ public Package finishBuild() { return pkg; } - // Freeze targets and distributions. + // Freeze rules, compacting their attributes' representations. for (Rule rule : getRules()) { rule.freeze(); } - ruleLabels = null; - rulesCreatedInMacros = null; - outputFilePrefixes = null; - targets = Maps.unmodifiableBiMap(targets); // Now all targets have been loaded, so we validate the group's member environments. for (EnvironmentGroup envGroup : ImmutableSet.copyOf(environmentGroups.values())) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java b/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java index 2bb0951ef69e33..e7b0e4d1021ee4 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/packages/TargetRegistrationEnvironment.java @@ -167,7 +167,7 @@ private enum NameConflictCheckingPolicy { *

This field is null if name conflict checking is disabled. It is also null after the package * is built. The content of the map is manipulated only in {@link #checkRuleAndOutputs}. */ - @Nullable protected Map outputFilePrefixes = new HashMap<>(); + @Nullable private Map outputFilePrefixes = new HashMap<>(); protected TargetRegistrationEnvironment(RepositoryMapping mainRepositoryMapping) { super(mainRepositoryMapping);