Report background download for BwoB

bazelbuild · Feb 27, 2023 · e0c65f6 · e0c65f6
1 parent 4a6d056
commit e0c65f6
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 18 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnProgressEvent.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnProgressEvent.java
@@ -28,13 +28,13 @@ public static SpawnProgressEvent create(String resourceId, String progress, bool
   }
 
   /** The id that uniquely determines the progress among all progress events for this spawn. */
-  abstract String progressId();
+  public abstract String progressId();
 
   /** Human readable description of the progress. */
-  abstract String progress();
+  public abstract String progress();
 
   /** Whether the progress reported about is finished already. */
-  abstract boolean finished();
+  public abstract boolean finished();
 
   @Override
   public void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action) {

diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
@@ -161,7 +161,11 @@ private boolean shouldDownloadFile(Path path, FileArtifactValue metadata) {
    * @param tempPath the temporary path which the input should be written to.
    */
   protected abstract ListenableFuture<Void> doDownloadFile(
-      Path tempPath, PathFragment execPath, FileArtifactValue metadata, Priority priority)
+      Reporter reporter,
+      Path tempPath,
+      PathFragment execPath,
+      FileArtifactValue metadata,
+      Priority priority)
       throws IOException;
 
   protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOException {}
@@ -321,7 +325,8 @@ private Completable prefetchInputTree(
                   return toTransferResult(
                       toCompletable(
                           () ->
-                              doDownloadFile(tempPath, treeFile.getExecPath(), metadata, priority),
+                              doDownloadFile(
+                                  reporter, tempPath, treeFile.getExecPath(), metadata, priority),
                           directExecutor()));
                 });
 
@@ -467,7 +472,11 @@ private Completable downloadFileNoCheckRx(
                     toCompletable(
                             () ->
                                 doDownloadFile(
-                                    tempPath, finalPath.relativeTo(execRoot), metadata, priority),
+                                    reporter,
+                                    tempPath,
+                                    finalPath.relativeTo(execRoot),
+                                    metadata,
+                                    priority),
                             directExecutor())
                         .doOnComplete(
                             () -> {

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
@@ -23,7 +23,10 @@
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
+import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
 import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.exec.SpawnProgressEvent;
+import com.google.devtools.build.lib.remote.RemoteCache.DownloadProgressReporter;
 import com.google.devtools.build.lib.remote.common.BulkTransferException;
 import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
 import com.google.devtools.build.lib.remote.util.DigestUtil;
@@ -87,15 +90,50 @@ protected boolean canDownloadFile(Path path, FileArtifactValue metadata) {
 
   @Override
   protected ListenableFuture<Void> doDownloadFile(
-      Path tempPath, PathFragment execPath, FileArtifactValue metadata, Priority priority)
+      Reporter reporter,
+      Path tempPath,
+      PathFragment execPath,
+      FileArtifactValue metadata,
+      Priority priority)
       throws IOException {
     checkArgument(metadata.isRemote(), "Cannot download file that is not a remote file.");
     RequestMetadata requestMetadata =
         TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "prefetcher", null);
     RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata);
 
     Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
-    return remoteCache.downloadFile(context, tempPath, digest);
+    return remoteCache.downloadFile(
+        context,
+        tempPath,
+        digest,
+        new DownloadProgressReporter(
+            /* includeFile= */ false,
+            progress -> reporter.post(new DownloadProgress(progress)),
+            execPath.toString(),
+            metadata.getSize()));
+  }
+
+  public static class DownloadProgress implements FetchProgress {
+    private final SpawnProgressEvent progress;
+
+    public DownloadProgress(SpawnProgressEvent progress) {
+      this.progress = progress;
+    }
+
+    @Override
+    public String getResourceIdentifier() {
+      return progress.progressId();
+    }
+
+    @Override
+    public String getProgress() {
+      return progress.progress();
+    }
+
+    @Override
+    public boolean isFinished() {
+      return progress.finished();
+    }
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -248,13 +248,20 @@ private ListenableFuture<Void> downloadBlob(
   /** A reporter that reports download progresses. */
   public static class DownloadProgressReporter {
     private static final Pattern PATTERN = Pattern.compile("^bazel-out/[^/]+/[^/]+/");
+    private final boolean includeFile;
     private final ProgressStatusListener listener;
     private final String id;
     private final String file;
     private final String totalSize;
     private final AtomicLong downloadedBytes = new AtomicLong(0);
 
     public DownloadProgressReporter(ProgressStatusListener listener, String file, long totalSize) {
+      this(/* includeFile= */ true, listener, file, totalSize);
+    }
+
+    public DownloadProgressReporter(
+        boolean includeFile, ProgressStatusListener listener, String file, long totalSize) {
+      this.includeFile = includeFile;
       this.listener = listener;
       this.id = file;
       this.totalSize = bytesCountToDisplayString(totalSize);
@@ -279,12 +286,21 @@ void finished() {
     private void reportProgress(boolean includeBytes, boolean finished) {
       String progress;
       if (includeBytes) {
-        progress =
-            String.format(
-                "Downloading %s, %s / %s",
-                file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
+        if (includeFile) {
+          progress =
+              String.format(
+                  "Downloading %s, %s / %s",
+                  file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
+        } else {
+          progress =
+              String.format("%s / %s", bytesCountToDisplayString(downloadedBytes.get()), totalSize);
+        }
       } else {
-        progress = String.format("Downloading %s", file);
+        if (includeFile) {
+          progress = String.format("Downloading %s", file);
+        } else {
+          progress = "";
+        }
       }
       listener.onProgressStatus(SpawnProgressEvent.create(id, progress, finished));
     }

diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/ProgressStatusListener.java b/src/main/java/com/google/devtools/build/lib/remote/common/ProgressStatusListener.java
@@ -13,13 +13,14 @@
 // limitations under the License.
 package com.google.devtools.build.lib.remote.common;
 
+import com.google.devtools.build.lib.exec.SpawnProgressEvent;
 import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
 
 /** An interface that is used to receive {@link ProgressStatus} updates during spawn execution. */
 @FunctionalInterface
 public interface ProgressStatusListener {
 
-  void onProgressStatus(ProgressStatus progress);
+  void onProgressStatus(SpawnProgressEvent progress);
 
   /** A {@link ProgressStatusListener} that does nothing. */
   ProgressStatusListener NO_ACTION =

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
@@ -672,7 +672,11 @@ public void afterCommand(AfterCommandEvent event) {
   public void downloadProgress(FetchProgress event) {
     maybeAddDate();
     stateTracker.downloadProgress(event);
-    refresh();
+    if (event.isFinished()) {
+      checkActivities();
+    } else {
+      refreshSoon();
+    }
   }
 
   @Subscribe

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java
@@ -993,7 +993,8 @@ synchronized boolean hasActivities() {
     return !(buildCompleted()
         && bepOpenTransports.isEmpty()
         && activeActionUploads.get() == 0
-        && activeActionDownloads.get() == 0);
+        && activeActionDownloads.get() == 0
+        && runningDownloads.isEmpty());
   }
 
   /**
@@ -1106,7 +1107,7 @@ private void reportOnOneDownload(
     terminalWriter.append(url + postfix);
   }
 
-  protected void reportOnDownloads(AnsiTerminalWriter terminalWriter) throws IOException {
+  protected void reportOnDownloads(PositionAwareAnsiTerminalWriter terminalWriter) throws IOException {
     int count = 0;
     long nanoTime = clock.nanoTime();
     int downloadCount = runningDownloads.size();
@@ -1116,7 +1117,10 @@ protected void reportOnDownloads(AnsiTerminalWriter terminalWriter) throws IOExc
         break;
       }
       count++;
-      terminalWriter.newline().append(FETCH_PREFIX);
+      if (terminalWriter.getPosition() != 0) {
+        terminalWriter.newline();
+      }
+      terminalWriter.append(FETCH_PREFIX);
       reportOnOneDownload(
           url,
           nanoTime,