From 49731660d8348d4ce243c4531bd5f3b14006f1df Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 21 Mar 2023 01:37:10 -0700 Subject: [PATCH] Add option to handle edges from file write actions (like) actions correctly. PiperOrigin-RevId: 518203038 Change-Id: I3c1e903a7ad0adb148e74b601383ad3308c250d5 --- .../java/com/google/devtools/build/lib/BUILD | 1 + .../includescanning/SpawnIncludeScanner.java | 2 +- .../lib/runtime/ExecutionGraphModule.java | 30 ++++++++++++++++++- .../lib/runtime/ExecutionGraphModuleTest.java | 14 ++++++--- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index b9fa8fc0baaa96..d580386e833cce 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -290,6 +290,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:package_roots", "//src/main/java/com/google/devtools/build/lib/actions:resource_manager", "//src/main/java/com/google/devtools/build/lib/actions:shared_action_event", + "//src/main/java/com/google/devtools/build/lib/analysis:actions/abstract_file_write_action", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_options", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_complete_event", diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java index 7528966da3f14d..baf09b188fbc4a 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java @@ -198,7 +198,7 @@ public NestedSet getTools() { @Override public NestedSet getInputs() { - throw new UnsupportedOperationException(); + return actionExecutionMetadata.getInputs(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ExecutionGraphModule.java b/src/main/java/com/google/devtools/build/lib/runtime/ExecutionGraphModule.java index b53fbb92bf6e9e..7dc358333b9981 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ExecutionGraphModule.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ExecutionGraphModule.java @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.actions.SpawnExecutedEvent; import com.google.devtools.build.lib.actions.SpawnMetrics; import com.google.devtools.build.lib.actions.SpawnResult; +import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileCompression; @@ -151,6 +152,14 @@ public static class ExecutionGraphOptions extends OptionsBase { defaultValue = "true", help = "Subscribe to ActionCompletionEvent in ExecutionGraphModule.") public boolean logMissedActions; + + @Option( + name = "experimental_execution_graph_enable_edges_from_filewrite_actions", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.UNKNOWN}, + defaultValue = "false", + help = "Handle edges from filewrite actions to their inputs correctly.") + public boolean logFileWriteEdges; } /** What level of dependency information to include in the dump. */ @@ -455,12 +464,22 @@ private ExecutionGraph.Node toProto(SpawnExecutedEvent event) { metricsBuilder.putRetryMillisByError(entry.getKey(), entry.getValue()); } metrics = null; + + NestedSet inputFiles; + if (logFileWriteEdges && spawn.getResourceOwner() instanceof AbstractFileWriteAction) { + // In order to handle file write like actions correctly, get the inputs + // from the corresponding action. + inputFiles = spawn.getResourceOwner().getInputs(); + } else { + inputFiles = spawn.getInputFiles(); + } + // maybeAddEdges can take a while, so do it last and try to give up references to any objects // we won't need. maybeAddEdges( nodeBuilder, spawn.getOutputEdgesForExecutionGraph(), - spawn.getInputFiles(), + inputFiles, spawn.getResourceOwner(), spawn.getRunfilesSupplier(), startMillis, @@ -578,6 +597,7 @@ private NodeInfo(int index, long finishMs) { private final BugReporter bugReporter; private final boolean localLockFreeOutputEnabled; + private final boolean logFileWriteEdges; private final Map outputToNode = new ConcurrentHashMap<>(); private final Map outputToDiscoverInputsTime = new ConcurrentHashMap<>(); private final DependencyInfo depType; @@ -605,12 +625,14 @@ private NodeInfo(int index, long finishMs) { ActionDumpWriter( BugReporter bugReporter, boolean localLockFreeOutputEnabled, + boolean logFileWriteEdges, OutputStream outStream, UUID commandId, DependencyInfo depType, int queueSize) { this.bugReporter = bugReporter; this.localLockFreeOutputEnabled = localLockFreeOutputEnabled; + this.logFileWriteEdges = logFileWriteEdges; this.outStream = outStream; this.depType = depType; if (queueSize < 0) { @@ -772,6 +794,7 @@ private ActionDumpWriter createActionDumpWriter(CommandEnvironment env) return new StreamingActionDumpWriter( env.getRuntime().getBugReporter(), env.getOptions().getOptions(LocalExecutionOptions.class).localLockfreeOutput, + executionGraphOptions.logFileWriteEdges, newUploader(env, bepOptions).startUpload(LocalFileType.PERFORMANCE_LOG, null), env.getCommandId(), executionGraphOptions.depType, @@ -784,6 +807,7 @@ private ActionDumpWriter createActionDumpWriter(CommandEnvironment env) return new FilesystemActionDumpWriter( env.getRuntime().getBugReporter(), env.getOptions().getOptions(LocalExecutionOptions.class).localLockfreeOutput, + executionGraphOptions.logFileWriteEdges, actionGraphFile, env.getCommandId(), executionGraphOptions.depType, @@ -799,6 +823,7 @@ private static final class FilesystemActionDumpWriter extends ActionDumpWriter { public FilesystemActionDumpWriter( BugReporter bugReporter, boolean localLockFreeOutputEnabled, + boolean logFileWriteEdges, Path actionGraphFile, UUID uuid, DependencyInfo depType, @@ -807,6 +832,7 @@ public FilesystemActionDumpWriter( super( bugReporter, localLockFreeOutputEnabled, + logFileWriteEdges, actionGraphFile.getOutputStream(), uuid, depType, @@ -845,6 +871,7 @@ private static class StreamingActionDumpWriter extends ActionDumpWriter { public StreamingActionDumpWriter( BugReporter bugReporter, boolean localLockFreeOutputEnabled, + boolean logFileWriteEdges, UploadContext uploadContext, UUID commandId, DependencyInfo depType, @@ -852,6 +879,7 @@ public StreamingActionDumpWriter( super( bugReporter, localLockFreeOutputEnabled, + logFileWriteEdges, uploadContext.getOutputStream(), commandId, depType, diff --git a/src/test/java/com/google/devtools/build/lib/runtime/ExecutionGraphModuleTest.java b/src/test/java/com/google/devtools/build/lib/runtime/ExecutionGraphModuleTest.java index fa710052323d3b..91cfa0b082bb84 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/ExecutionGraphModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/ExecutionGraphModuleTest.java @@ -301,7 +301,8 @@ public void failureInOutputDoesNotHang( ActionDumpWriter writer = new ActionDumpWriter( BugReporter.defaultInstance(), - /*localLockFreeOutputEnabled=*/ false, + /* localLockFreeOutputEnabled= */ false, + /* logFileWriteEdges= */ false, OutputStream.nullOutputStream(), uuid, DependencyInfo.NONE, @@ -326,7 +327,8 @@ private void startLogging( startLogging( eventBus, BugReporter.defaultInstance(), - /*localLockFreeOutputEnabled=*/ false, + /* localLockFreeOutputEnabled= */ false, + /* logFileWriteEdges= */ false, uuid, buffer, depType); @@ -336,11 +338,13 @@ private void startLogging( EventBus eventBus, BugReporter bugReporter, boolean localLockFreeOutputEnabled, + boolean logFileWriteEdges, UUID uuid, OutputStream buffer, DependencyInfo depType) { ActionDumpWriter writer = - new ActionDumpWriter(bugReporter, localLockFreeOutputEnabled, buffer, uuid, depType, -1) { + new ActionDumpWriter( + bugReporter, localLockFreeOutputEnabled, logFileWriteEdges, buffer, uuid, depType, -1) { @Override protected void updateLogs(BuildToolLogCollection logs) {} }; @@ -541,6 +545,7 @@ public void multipleSpawnsWithSameOutput_overlapping_recordsBothSpawnsWithoutRet eventBus, bugReporter, localLockFreeOutput.optionValue, + /* logFileWriteEdges= */ false, UUID.randomUUID(), buffer, DependencyInfo.ALL); @@ -585,7 +590,8 @@ public void multipleSpawnsWithSameOutput_overlapping_ignoresSecondSpawnForDepend startLogging( eventBus, BugReporter.defaultInstance(), - /*localLockFreeOutputEnabled=*/ true, + /* localLockFreeOutputEnabled= */ true, + /* logFileWriteEdges= */ false, UUID.randomUUID(), buffer, DependencyInfo.ALL);