From 2c8226018ef196bfaad5235b65ca9073f5637242 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 23 Aug 2024 10:10:35 +0200 Subject: [PATCH] Address comments --- .../devtools/build/lib/actions/ActionExecutionMetadata.java | 2 +- .../google/devtools/build/lib/remote/RemoteSpawnCache.java | 6 +++--- .../devtools/build/lib/rules/java/JavaCompileAction.java | 2 +- .../build/lib/rules/java/JavaHeaderCompileAction.java | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java index 36ce7648be6aff..eb9cc3d0e68897 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java @@ -116,7 +116,7 @@ default boolean mayInsensitivelyPropagateInputs() { } /** - * Returns true if the action may modify its outputs after executing spawns. + * Returns true if the action may modify spawn outputs after the spawn has executed. * *

If this returns true, any kind of spawn output caching or reuse needs to happen * synchronously directly after the spawn execution. diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 7335bc3ff7d562..ae802b93203935 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -129,8 +129,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) previousExecution = previousOrThisExecution == thisExecution ? null : previousOrThisExecution; } - // Metadata will be available in context.current() until we detach. - // This is done via a thread-local variable. try { RemoteActionResult result; try (SilentCloseable c = @@ -269,7 +267,9 @@ public void store(SpawnResult result) throws ExecException, InterruptedException // existing executions finish the reuse. // Note that while this call itself isn't interruptible, all operations it awaits are // interruptible. - thisExecutionFinal.awaitAllOutputReuse(); + try (SilentCloseable c = prof.profile(REMOTE_DOWNLOAD, "await output reuse")) { + thisExecutionFinal.awaitAllOutputReuse(); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index 3b011d581d2b5a..5db91c5f75888b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -713,7 +713,7 @@ public NestedSet getPossibleInputsForTesting() { public boolean mayModifySpawnOutputsAfterExecution() { // Causes of spawn output modification after execution: // - Fallback to the full classpath with --experimental_java_classpath=bazel. - // - In-place rewriting of .jdeps files when with --experimental_output_paths=strip. + // - In-place rewriting of .jdeps files with --experimental_output_paths=strip. return true; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java index 83f8922afa2118..ff7db176c273a1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java @@ -160,7 +160,7 @@ protected void afterExecute( @Override public boolean mayModifySpawnOutputsAfterExecution() { // Causes of spawn output modification after execution: - // - In-place rewriting of .jdeps files when with --experimental_output_paths=strip. + // - In-place rewriting of .jdeps files with --experimental_output_paths=strip. // TODO: Use separate files as action and spawn output to avoid in-place modification. return true; }