Skip to content

Commit 0a23a5e

Browse files
janakdrcopybara-github
authored andcommitted
Log source artifact bytes read during this build to BEP.
Suggested by @jfield. PiperOrigin-RevId: 353789327
1 parent 77a980f commit 0a23a5e

File tree

9 files changed

+107
-61
lines changed

9 files changed

+107
-61
lines changed

src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,10 @@ message BuildMetrics {
740740
// This includes any remote cache hits, but excludes
741741
// local action cache hits.
742742
int64 actions_executed = 2;
743+
744+
// Total size of all source files newly read this build. Will not include
745+
// unchanged sources on incremental builds.
746+
int64 source_artifact_bytes_read = 5;
743747
}
744748
ActionSummary action_summary = 1;
745749

src/main/java/com/google/devtools/build/lib/metrics/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ java_library(
3838
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_started_event",
3939
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
4040
"//src/main/java/com/google/devtools/build/lib/profiler",
41+
"//src/main/java/com/google/devtools/build/lib/skyframe:execution_finished_event",
4142
"//src/main/java/com/google/devtools/common/options",
4243
"//third_party:guava",
4344
],

src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.google.devtools.build.lib.metrics.PostGCMemoryUseRecorder.PeakHeap;
3232
import com.google.devtools.build.lib.profiler.Profiler;
3333
import com.google.devtools.build.lib.runtime.CommandEnvironment;
34+
import com.google.devtools.build.lib.skyframe.ExecutionFinishedEvent;
3435
import java.lang.management.ManagementFactory;
3536
import java.lang.management.MemoryMXBean;
3637
import java.time.Duration;
@@ -49,7 +50,8 @@ class MetricsCollector {
4950
private int targetsLoaded;
5051
private int targetsConfigured;
5152
private int packagesLoaded;
52-
private static long analysisTimeInMs;
53+
private long sourceArtifactBytesReadPerBuild;
54+
private long analysisTimeInMs;
5355

5456
private MetricsCollector(CommandEnvironment env) {
5557
this.env = env;
@@ -92,6 +94,12 @@ public void onActionComplete(ActionCompletionEvent event) {
9294
executedActionCount.incrementAndGet();
9395
}
9496

97+
@SuppressWarnings("unused")
98+
@Subscribe
99+
public void onExecutionComplete(ExecutionFinishedEvent event) {
100+
this.sourceArtifactBytesReadPerBuild = event.sourceArtifactBytesRead();
101+
}
102+
95103
@Subscribe
96104
public void onBuildComplete(BuildPrecompleteEvent event) {
97105
env.getEventBus().post(new BuildMetricsEvent(createBuildMetrics()));
@@ -112,6 +120,7 @@ private ActionSummary createActionSummary() {
112120
return ActionSummary.newBuilder()
113121
.setActionsCreated(actionsConstructed)
114122
.setActionsExecuted(executedActionCount.get())
123+
.setSourceArtifactBytesRead(sourceArtifactBytesReadPerBuild)
115124
.build();
116125
}
117126

@@ -147,7 +156,7 @@ private CumulativeMetrics createCumulativeMetrics() {
147156
.build();
148157
}
149158

150-
private static TimingMetrics createTimingMetrics() {
159+
private TimingMetrics createTimingMetrics() {
151160
TimingMetrics.Builder timingMetricsBuilder = TimingMetrics.newBuilder();
152161
Duration elapsedWallTime = Profiler.elapsedTimeMaybe();
153162
if (elapsedWallTime != null) {

src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java

Lines changed: 49 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import java.util.Comparator;
5555
import java.util.List;
5656
import java.util.Map;
57+
import java.util.concurrent.atomic.AtomicLong;
5758
import java.util.function.Supplier;
5859
import javax.annotation.Nullable;
5960

@@ -65,6 +66,7 @@
6566
*/
6667
class ArtifactFunction implements SkyFunction {
6768
private final Supplier<Boolean> mkdirForTreeArtifacts;
69+
private final AtomicLong sourceArtifactBytesReadThisBuild;
6870

6971
public static final class MissingFileArtifactValue implements SkyValue {
7072
private final DetailedExitCode detailedExitCode;
@@ -87,8 +89,10 @@ public String toString() {
8789
}
8890
}
8991

90-
public ArtifactFunction(Supplier<Boolean> mkdirForTreeArtifacts) {
92+
public ArtifactFunction(
93+
Supplier<Boolean> mkdirForTreeArtifacts, AtomicLong sourceArtifactBytesReadThisBuild) {
9194
this.mkdirForTreeArtifacts = mkdirForTreeArtifacts;
95+
this.sourceArtifactBytesReadThisBuild = sourceArtifactBytesReadThisBuild;
9296
}
9397

9498
@Override
@@ -251,7 +255,7 @@ private static TreeArtifactValue createTreeArtifactValueFromActionKey(
251255
return tree;
252256
}
253257

254-
private static SkyValue createSourceValue(Artifact artifact, Environment env)
258+
private SkyValue createSourceValue(Artifact artifact, Environment env)
255259
throws IOException, InterruptedException {
256260
RootedPath path = RootedPath.toRootedPath(artifact.getRoot().getRoot(), artifact.getPath());
257261
SkyKey fileSkyKey = FileValue.key(path);
@@ -268,51 +272,54 @@ private static SkyValue createSourceValue(Artifact artifact, Environment env)
268272
return makeMissingSourceInputFileValue(artifact);
269273
}
270274

271-
// For directory artifacts that are not Filesets, we initiate a directory traversal here, and
272-
// compute a hash from the directory structure.
273-
if (fileValue.isDirectory() && TrackSourceDirectoriesFlag.trackSourceDirectories()) {
274-
// We rely on the guarantees of RecursiveFilesystemTraversalFunction for correctness.
275-
//
276-
// This approach may have unexpected interactions with --package_path. In particular, the exec
277-
// root is setup from the loading / analysis phase, and it is now too late to change it;
278-
// therefore, this may traverse a different set of files depending on which targets are built
279-
// at the same time and what the package-path layout is (this may be moot if there is only one
280-
// entry). Or this may return a set of files that's inconsistent with those actually available
281-
// to the action (for local execution).
282-
//
283-
// In the future, we need to make this result the source of truth for the files available to
284-
// the action so that we at least have consistency.
285-
TraversalRequest request =
286-
TraversalRequest.create(
287-
DirectTraversalRoot.forRootedPath(path),
288-
/*isRootGenerated=*/ false,
289-
PackageBoundaryMode.CROSS,
290-
/*strictOutputFiles=*/ true,
291-
/*skipTestingForSubpackage=*/ true,
292-
/*errorInfo=*/ "Directory artifact " + artifact.prettyPrint());
293-
RecursiveFilesystemTraversalValue value;
294-
try {
295-
value =
296-
(RecursiveFilesystemTraversalValue) env.getValueOrThrow(
297-
request, RecursiveFilesystemTraversalException.class);
298-
} catch (RecursiveFilesystemTraversalException e) {
299-
throw new IOException(e);
300-
}
301-
if (value == null) {
302-
return null;
275+
if (!fileValue.isDirectory() || !TrackSourceDirectoriesFlag.trackSourceDirectories()) {
276+
if (fileValue.isFile()) {
277+
sourceArtifactBytesReadThisBuild.addAndGet(fileValue.getSize());
303278
}
304-
Fingerprint fp = new Fingerprint();
305-
for (ResolvedFile file : value.getTransitiveFiles().toList()) {
306-
fp.addString(file.getNameInSymlinkTree().getPathString());
307-
fp.addBytes(file.getMetadata().getDigest());
279+
try {
280+
return FileArtifactValue.createForSourceArtifact(artifact, fileValue);
281+
} catch (IOException e) {
282+
return makeIOExceptionSourceInputFileValue(artifact, e);
308283
}
309-
return FileArtifactValue.createForDirectoryWithHash(fp.digestAndReset());
310284
}
285+
// For directory artifacts that are not Filesets, we initiate a directory traversal here, and
286+
// compute a hash from the directory structure.
287+
// We rely on the guarantees of RecursiveFilesystemTraversalFunction for correctness.
288+
//
289+
// This approach may have unexpected interactions with --package_path. In particular, the exec
290+
// root is setup from the loading / analysis phase, and it is now too late to change it;
291+
// therefore, this may traverse a different set of files depending on which targets are built
292+
// at the same time and what the package-path layout is (this may be moot if there is only one
293+
// entry). Or this may return a set of files that's inconsistent with those actually available
294+
// to the action (for local execution).
295+
//
296+
// In the future, we need to make this result the source of truth for the files available to
297+
// the action so that we at least have consistency.
298+
TraversalRequest request =
299+
TraversalRequest.create(
300+
DirectTraversalRoot.forRootedPath(path),
301+
/*isRootGenerated=*/ false,
302+
PackageBoundaryMode.CROSS,
303+
/*strictOutputFiles=*/ true,
304+
/*skipTestingForSubpackage=*/ true,
305+
/*errorInfo=*/ "Directory artifact " + artifact.prettyPrint());
306+
RecursiveFilesystemTraversalValue value;
311307
try {
312-
return FileArtifactValue.createForSourceArtifact(artifact, fileValue);
313-
} catch (IOException e) {
314-
return makeIOExceptionSourceInputFileValue(artifact, e);
308+
value =
309+
(RecursiveFilesystemTraversalValue)
310+
env.getValueOrThrow(request, RecursiveFilesystemTraversalException.class);
311+
} catch (RecursiveFilesystemTraversalException e) {
312+
throw new IOException(e);
313+
}
314+
if (value == null) {
315+
return null;
316+
}
317+
Fingerprint fp = new Fingerprint();
318+
for (ResolvedFile file : value.getTransitiveFiles().toList()) {
319+
fp.addString(file.getNameInSymlinkTree().getPathString());
320+
fp.addBytes(file.getMetadata().getDigest());
315321
}
322+
return FileArtifactValue.createForDirectoryWithHash(fp.digestAndReset());
316323
}
317324

318325
static MissingFileArtifactValue makeMissingSourceInputFileValue(Artifact artifact) {

src/main/java/com/google/devtools/build/lib/skyframe/ExecutionFinishedEvent.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,16 @@
2222
*/
2323
@AutoValue
2424
public abstract class ExecutionFinishedEvent {
25-
public static final ExecutionFinishedEvent EMPTY =
26-
builder()
27-
.setOutputDirtyFiles(0)
28-
.setOutputModifiedFilesDuringPreviousBuild(0)
29-
.setSourceDiffCheckingDuration(Duration.ZERO)
30-
.setNumSourceFilesCheckedBecauseOfMissingDiffs(0)
31-
.setOutputTreeDiffCheckingDuration(Duration.ZERO)
32-
.build();
25+
// AutoValue Builders require that all fields are populated, so we provide a default.
26+
public static ExecutionFinishedEvent.Builder builderWithDefaults() {
27+
return builder()
28+
.setOutputDirtyFiles(0)
29+
.setOutputModifiedFilesDuringPreviousBuild(0)
30+
.setSourceDiffCheckingDuration(Duration.ZERO)
31+
.setNumSourceFilesCheckedBecauseOfMissingDiffs(0)
32+
.setOutputTreeDiffCheckingDuration(Duration.ZERO)
33+
.setSourceArtifactBytesRead(0L);
34+
}
3335

3436
public abstract int outputDirtyFiles();
3537

@@ -41,6 +43,8 @@ public abstract class ExecutionFinishedEvent {
4143

4244
public abstract Duration outputTreeDiffCheckingDuration();
4345

46+
public abstract long sourceArtifactBytesRead();
47+
4448
static Builder builder() {
4549
return new AutoValue_ExecutionFinishedEvent.Builder();
4650
}
@@ -59,6 +63,8 @@ abstract Builder setNumSourceFilesCheckedBecauseOfMissingDiffs(
5963

6064
abstract Builder setOutputTreeDiffCheckingDuration(Duration outputTreeDiffCheckingDuration);
6165

66+
abstract Builder setSourceArtifactBytesRead(long sourceArtifactBytesRead);
67+
6268
abstract ExecutionFinishedEvent build();
6369
}
6470
}

src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -912,21 +912,20 @@ public Void call() throws InterruptedException {
912912
}
913913

914914
@Override
915-
public ExecutionFinishedEvent createExecutionFinishedEvent() {
916-
ExecutionFinishedEvent result =
915+
protected ExecutionFinishedEvent.Builder createExecutionFinishedEventInternal() {
916+
ExecutionFinishedEvent.Builder builder =
917917
ExecutionFinishedEvent.builder()
918918
.setOutputDirtyFiles(outputDirtyFiles)
919919
.setOutputModifiedFilesDuringPreviousBuild(modifiedFilesDuringPreviousBuild)
920920
.setSourceDiffCheckingDuration(sourceDiffCheckingDuration)
921921
.setNumSourceFilesCheckedBecauseOfMissingDiffs(
922922
numSourceFilesCheckedBecauseOfMissingDiffs)
923-
.setOutputTreeDiffCheckingDuration(outputTreeDiffCheckingDuration)
924-
.build();
923+
.setOutputTreeDiffCheckingDuration(outputTreeDiffCheckingDuration);
925924
outputDirtyFiles = 0;
926925
modifiedFilesDuringPreviousBuild = 0;
927926
sourceDiffCheckingDuration = Duration.ZERO;
928927
outputTreeDiffCheckingDuration = Duration.ZERO;
929-
return result;
928+
return builder;
930929
}
931930

932931
@Override

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@
217217
import java.util.concurrent.Semaphore;
218218
import java.util.concurrent.atomic.AtomicBoolean;
219219
import java.util.concurrent.atomic.AtomicInteger;
220+
import java.util.concurrent.atomic.AtomicLong;
220221
import java.util.concurrent.atomic.AtomicReference;
221222
import java.util.function.Consumer;
222223
import java.util.function.Supplier;
@@ -249,6 +250,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory, Configur
249250
protected final BlazeDirectories directories;
250251
protected final ExternalFilesHelper externalFilesHelper;
251252
private final GraphInconsistencyReceiver graphInconsistencyReceiver;
253+
/**
254+
* Tracks the accumulated size of source artifacts read this build. Does not include cached
255+
* artifacts, so is not useful on incremental builds.
256+
*/
257+
private final AtomicLong sourceArtifactBytesReadThisBuild = new AtomicLong();
258+
252259
@Nullable protected OutputService outputService;
253260

254261
// TODO(bazel-team): Figure out how to handle value builders that block internally. Blocking
@@ -584,7 +591,8 @@ private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions(PackageFactory p
584591
map.put(
585592
Artifact.ARTIFACT,
586593
new ArtifactFunction(
587-
() -> !skyframeActionExecutor.actionFileSystemType().inMemoryFileSystem()));
594+
() -> !skyframeActionExecutor.actionFileSystemType().inMemoryFileSystem(),
595+
sourceArtifactBytesReadThisBuild));
588596
map.put(
589597
SkyFunctions.BUILD_INFO_COLLECTION,
590598
new BuildInfoCollectionFunction(actionKeyContext, artifactFactory));
@@ -2694,6 +2702,7 @@ protected final void syncPackageLoadingInternal(
26942702

26952703
incrementalBuildMonitor = new SkyframeIncrementalBuildMonitor();
26962704
invalidateTransientErrors();
2705+
sourceArtifactBytesReadThisBuild.set(0L);
26972706
}
26982707

26992708
private void getActionEnvFromOptions(CoreOptions opt) {
@@ -3014,7 +3023,16 @@ public void evaluated(
30143023
}
30153024
}
30163025

3017-
public abstract ExecutionFinishedEvent createExecutionFinishedEvent();
3026+
public final ExecutionFinishedEvent createExecutionFinishedEvent() {
3027+
return createExecutionFinishedEventInternal()
3028+
.setSourceArtifactBytesRead(sourceArtifactBytesReadThisBuild.getAndSet(0L))
3029+
.build();
3030+
}
3031+
3032+
@ForOverride
3033+
protected ExecutionFinishedEvent.Builder createExecutionFinishedEventInternal() {
3034+
return ExecutionFinishedEvent.builderWithDefaults();
3035+
}
30183036

30193037
protected Iterable<ActionLookupValue> getActionLookupValuesInBuild(
30203038
List<ConfiguredTargetKey> topLevelCtKeys, List<AspectValueKey> aspectKeys)

src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import java.io.IOException;
5353
import java.util.LinkedHashSet;
5454
import java.util.UUID;
55+
import java.util.concurrent.atomic.AtomicLong;
5556
import java.util.concurrent.atomic.AtomicReference;
5657
import org.junit.Before;
5758

@@ -106,7 +107,7 @@ public void baseSetUp() throws Exception {
106107
new AtomicReference<>(UnixGlob.DEFAULT_SYSCALLS),
107108
externalFilesHelper))
108109
.put(FileValue.FILE, new FileFunction(pkgLocator))
109-
.put(Artifact.ARTIFACT, new ArtifactFunction(() -> true))
110+
.put(Artifact.ARTIFACT, new ArtifactFunction(() -> true, new AtomicLong()))
110111
.put(SkyFunctions.ACTION_EXECUTION, new SimpleActionExecutionFunction())
111112
.put(
112113
SkyFunctions.PACKAGE,

src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
import java.util.Map;
127127
import java.util.Set;
128128
import java.util.UUID;
129+
import java.util.concurrent.atomic.AtomicLong;
129130
import java.util.concurrent.atomic.AtomicReference;
130131
import javax.annotation.Nullable;
131132
import org.junit.Before;
@@ -260,7 +261,7 @@ protected BuilderWithResult createBuilder(
260261
new AtomicReference<>(UnixGlob.DEFAULT_SYSCALLS),
261262
externalFilesHelper))
262263
.put(FileValue.FILE, new FileFunction(pkgLocator))
263-
.put(Artifact.ARTIFACT, new ArtifactFunction(() -> true))
264+
.put(Artifact.ARTIFACT, new ArtifactFunction(() -> true, new AtomicLong()))
264265
.put(
265266
SkyFunctions.ACTION_EXECUTION,
266267
new ActionExecutionFunction(skyframeActionExecutor, directories, tsgmRef))

0 commit comments

Comments
 (0)