From e0c65f60372a7e9c378bdc05c729eaab39bd6e6c Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Mon, 27 Feb 2023 17:02:20 +0100 Subject: [PATCH] Report background download for BwoB --- .../build/lib/exec/SpawnProgressEvent.java | 6 +-- .../remote/AbstractActionInputPrefetcher.java | 15 +++++-- .../lib/remote/RemoteActionInputFetcher.java | 42 ++++++++++++++++++- .../build/lib/remote/RemoteCache.java | 26 +++++++++--- .../remote/common/ProgressStatusListener.java | 3 +- .../build/lib/runtime/UiEventHandler.java | 6 ++- .../build/lib/runtime/UiStateTracker.java | 10 +++-- 7 files changed, 90 insertions(+), 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 index 9ddde03a1e1497..4f4b139a576be5 100644 --- 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 index b11515ed40b0e7..027188364cc6e1 100644 --- 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 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 index f5ed91c94bb887..a637cc55b471f3 100644 --- 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,7 +90,11 @@ protected boolean canDownloadFile(Path path, FileArtifactValue metadata) { @Override protected ListenableFuture 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 = @@ -95,7 +102,38 @@ protected ListenableFuture doDownloadFile( 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 index f081add8ad9caa..1fe79e16906f0b 100644 --- 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,6 +248,7 @@ private ListenableFuture 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; @@ -255,6 +256,12 @@ public static class DownloadProgressReporter { 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 index df5a8beae54758..55b4b57f7c669f 100644 --- 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 index 5bc409de98f3e4..2a240993aa0890 100644 --- 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 index 718bda9d770716..db3c4fdb313807 100644 --- 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,