Skip to content

Commit

Permalink
[8.1.0] Reduce retained memory of UnresolvedSymlinkAction (#24873)
Browse files Browse the repository at this point in the history
The required conversion to `PathFragment` can be performed during
execution, which avoids retaining a new `PathFragment` instance per
action.

Closes #24831.

PiperOrigin-RevId: 713577830
Change-Id: I2781ea979a3c97abf5a519acd5d3d6a668dd1f77

Commit
293be8a

Co-authored-by: Fabian Meumertzheim <[email protected]>
Co-authored-by: Ian (Hee) Cha <[email protected]>
  • Loading branch information
3 people authored Jan 20, 2025
1 parent e9abd57 commit b1bd09c
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,13 @@
public final class UnresolvedSymlinkAction extends AbstractAction {
private static final String GUID = "0f302651-602c-404b-881c-58913193cfe7";

private final PathFragment target;
private final String target;
private final String progressMessage;

private UnresolvedSymlinkAction(
ActionOwner owner, Artifact primaryOutput, String target, String progressMessage) {
super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), ImmutableSet.of(primaryOutput));
// TODO: PathFragment#create normalizes the symlink target, which may change how it resolves
// when combined with directory symlinks. Ideally, Bazel's file system abstraction would
// offer a way to create symlinks without any preprocessing of the target.
this.target = PathFragment.create(target);
this.target = target;
this.progressMessage = progressMessage;
}

Expand All @@ -71,7 +68,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)

Path outputPath = actionExecutionContext.getInputPath(getPrimaryOutput());
try {
outputPath.createSymbolicLink(target);
outputPath.createSymbolicLink(getTargetPathFragment());
} catch (IOException e) {
String message =
String.format(
Expand All @@ -90,7 +87,7 @@ protected void computeKey(
@Nullable ArtifactExpander artifactExpander,
Fingerprint fp) {
fp.addString(GUID);
fp.addPath(target);
fp.addString(target);
}

@Override
Expand All @@ -108,8 +105,15 @@ protected String getRawProgressMessage() {
return progressMessage;
}

public PathFragment getTarget() {
return target;
public String getTarget() {
return getTargetPathFragment().getPathString();
}

private PathFragment getTargetPathFragment() {
// TODO: PathFragment#create normalizes the symlink target, which may change how it resolves
// when combined with directory symlinks. Ideally, Bazel's file system abstraction would
// offer a way to create symlinks without any preprocessing of the target.
return PathFragment.create(target);
}

private static DetailedExitCode createDetailedExitCode(String message, Code detailedCode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,7 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM


if (action instanceof UnresolvedSymlinkAction) {
actionBuilder.setUnresolvedSymlinkTarget(
((UnresolvedSymlinkAction) action).getTarget().toString());
actionBuilder.setUnresolvedSymlinkTarget(((UnresolvedSymlinkAction) action).getTarget());
}

// Include the content of param files in output.
Expand Down

0 comments on commit b1bd09c

Please sign in to comment.