Skip to content

Commit

Permalink
Don't zero-out builder fields in finishBuild()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
brandjon authored and copybara-github committed Sep 24, 2024
1 parent b58036e commit 8977891
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ private enum NameConflictCheckingPolicy {
* <p>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<String, OutputFile> outputFilePrefixes = new HashMap<>();
@Nullable private Map<String, OutputFile> outputFilePrefixes = new HashMap<>();

protected TargetRegistrationEnvironment(RepositoryMapping mainRepositoryMapping) {
super(mainRepositoryMapping);
Expand Down

0 comments on commit 8977891

Please sign in to comment.