Skip to content

Commit 721c854

Browse files
lberkicopybara-github
lberki
authored and
copybara-github
committed
Separate out the logic to construct a RemoteAnalysisCacheManager from the class itself.
Also remove DisabledDependenciesProvider. This is not necessary anymore how that the logic is clear enough to just add state checks. RELNOTES: None. PiperOrigin-RevId: 884496629 Change-Id: Ic8f648b2c742dcbb715b8cff0f68793a811a33bc
1 parent 94c043e commit 721c854

File tree

16 files changed

+558
-560
lines changed

16 files changed

+558
-560
lines changed

‎src/main/java/com/google/devtools/build/lib/analysis/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,8 @@ java_library(
558558
"//src/main/java/com/google/devtools/build/lib/skyframe:target_pattern_phase_value",
559559
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:dependencies_provider",
560560
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:options",
561+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_deps",
562+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_manager",
561563
"//src/main/java/com/google/devtools/build/lib/util",
562564
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
563565
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",

‎src/main/java/com/google/devtools/build/lib/analysis/BuildView.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@
9292
import com.google.devtools.build.lib.skyframe.SkyframeBuildView.BuildDriverKeyTestContext;
9393
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
9494
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
95+
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheDeps;
96+
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheManager;
9597
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheReaderDepsProvider;
9698
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider;
97-
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider.DisabledDependenciesProvider;
9899
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingOptions.RemoteAnalysisCacheMode;
99100
import com.google.devtools.build.lib.util.AbruptExitException;
100101
import com.google.devtools.build.lib.util.DetailedExitCode;
@@ -288,9 +289,10 @@ public AnalysisResult update(
288289
try (SilentCloseable c = Profiler.instance().profile("skycache.metadataQuery")) {
289290
remoteAnalysisCachingDependenciesProvider.queryMetadataAndMaybeBailout();
290291
}
291-
if (remoteAnalysisCachingDependenciesProvider.bailedOut()) {
292-
remoteAnalysisCachingDependenciesProvider = DisabledDependenciesProvider.INSTANCE;
293-
remoteAnalysisCacheReaderDeps = DisabledDependenciesProvider.INSTANCE;
292+
if (remoteAnalysisCachingDependenciesProvider.mode() != RemoteAnalysisCacheMode.OFF
293+
&& remoteAnalysisCachingDependenciesProvider.bailedOut()) {
294+
remoteAnalysisCachingDependenciesProvider = RemoteAnalysisCacheManager.createDisabled();
295+
remoteAnalysisCacheReaderDeps = RemoteAnalysisCacheDeps.createDisabled();
294296
} else {
295297
eventBus.post(new RemoteAnalysisCachingEnabledEvent());
296298
}

‎src/main/java/com/google/devtools/build/lib/buildtool/BUILD‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ java_library(
237237
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:frontier_serializer",
238238
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:options",
239239
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_client",
240-
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_manager",
240+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_factory",
241241
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_json_log_writer",
242242
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_metadata_writer",
243243
"//src/main/java/com/google/devtools/build/lib/util",

‎src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@
102102
import com.google.devtools.build.lib.skyframe.serialization.SkycacheMetadataParams;
103103
import com.google.devtools.build.lib.skyframe.serialization.analysis.FrontierSerializer;
104104
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheClient;
105-
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheManager;
105+
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheFactory;
106106
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheReaderDepsProvider;
107107
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider;
108108
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider.SerializationDependenciesProvider;
@@ -366,7 +366,7 @@ public void buildTargets(
366366
applyHeuristicInstrumentationFilter(buildOptions, targetPatternPhaseValue);
367367
}
368368
var analysisDeps =
369-
RemoteAnalysisCacheManager.create(
369+
RemoteAnalysisCacheFactory.create(
370370
env,
371371
projectEvaluationResult.activeDirectoriesMatcher(),
372372
targetPatternPhaseValue.getTargetLabels(),
@@ -439,13 +439,15 @@ public void buildTargets(
439439
reportOnlyBailOutReason(analysisCacheReaderDeps);
440440
} else {
441441
logAnalysisCachingStats(analysisCacheReaderDeps);
442-
RemoteAnalysisJsonLogWriter logWriter =
443-
serializationDependenciesProvider.getJsonLogWriter();
444-
if (logWriter != null) {
445-
logWriter.close();
446-
if (logWriter.hadErrors()) {
447-
env.getReporter()
448-
.handle(Event.warn("Skycache JSON log writing had errors, check Java logs"));
442+
if (analysisCacheReaderDeps.mode() != RemoteAnalysisCacheMode.OFF) {
443+
RemoteAnalysisJsonLogWriter logWriter =
444+
serializationDependenciesProvider.getJsonLogWriter();
445+
if (logWriter != null) {
446+
logWriter.close();
447+
if (logWriter.hadErrors()) {
448+
env.getReporter()
449+
.handle(Event.warn("Skycache JSON log writing had errors, check Java logs"));
450+
}
449451
}
450452
}
451453
}

‎src/main/java/com/google/devtools/build/lib/skyframe/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,8 @@ java_library(
353353
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:client_id",
354354
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:dependencies_provider",
355355
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:options",
356+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_deps",
357+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_manager",
356358
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_caching_server_state",
357359
"//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:platform_lookup_util",
358360
"//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:registered_execution_platforms_cycle_reporter",

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,10 @@
233233
import com.google.devtools.build.lib.skyframe.serialization.DeserializedSkyValue;
234234
import com.google.devtools.build.lib.skyframe.serialization.FrontierNodeVersion;
235235
import com.google.devtools.build.lib.skyframe.serialization.analysis.ClientId;
236+
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheDeps;
237+
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheManager;
236238
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheReaderDepsProvider;
237239
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider;
238-
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingDependenciesProvider.DisabledDependenciesProvider;
239240
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingOptions.RemoteAnalysisCacheMode;
240241
import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingServerState;
241242
import com.google.devtools.build.lib.skyframe.toolchains.RegisteredExecutionPlatformsCycleReporter;
@@ -527,11 +528,11 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
527528
private boolean remoteAnalysisCachingHasEverBeenEnabled = false;
528529

529530
private RemoteAnalysisCachingDependenciesProvider remoteAnalysisCachingDependenciesProvider =
530-
DisabledDependenciesProvider.INSTANCE;
531+
RemoteAnalysisCacheManager.createDisabled();
531532

532533
@Nullable
533534
private RemoteAnalysisCacheReaderDepsProvider remoteAnalysisCacheReaderDepsProvider =
534-
DisabledDependenciesProvider.INSTANCE;
535+
RemoteAnalysisCacheDeps.createDisabled();
535536

536537
/**
537538
* The state of the remote analysis caching.
@@ -559,12 +560,6 @@ public int getAndIncrementAnalysisCount() {
559560
* Returns the dependencies for remote analysis caching.
560561
*
561562
* <p>Should not be called before analysis begins.
562-
*
563-
* <p>This will reture {@link DisabledDependenciesProvider} until the top level configuration is
564-
* determined at the beginning of the analysis Skyframe evaluation, because it contains that
565-
* value. See the callsite of {@link
566-
* #setRemoteAnalysisCachingDependenciesProvider(RemoteAnalysisCachingDependenciesProvider)} for
567-
* the exact point.
568563
*/
569564
@VisibleForTesting // productionVisibility = Visibility.PRIVATE
570565
public RemoteAnalysisCachingDependenciesProvider getRemoteAnalysisCachingDependenciesProvider() {
@@ -1069,8 +1064,7 @@ public void setOutputService(@Nullable OutputService outputService) {
10691064
/** Inform this SkyframeExecutor that a new command is starting. */
10701065
public void noteCommandStart() {
10711066
// Prevent stale Skycache configuration from persisting between builds.
1072-
remoteAnalysisCachingDependenciesProvider =
1073-
RemoteAnalysisCachingDependenciesProvider.DisabledDependenciesProvider.INSTANCE;
1067+
remoteAnalysisCachingDependenciesProvider = RemoteAnalysisCacheManager.createDisabled();
10741068
}
10751069

10761070
/**
@@ -1201,8 +1195,7 @@ public void resetEvaluator() {
12011195
skyframeBuildView.reset();
12021196
// Prevent stale Skycache configuration from persisting between cleans.
12031197
remoteAnalysisCachingState = RemoteAnalysisCachingServerState.initializeEmpty();
1204-
remoteAnalysisCachingDependenciesProvider =
1205-
RemoteAnalysisCachingDependenciesProvider.DisabledDependenciesProvider.INSTANCE;
1198+
remoteAnalysisCachingDependenciesProvider = RemoteAnalysisCacheManager.createDisabled();
12061199
skyfocusState = DISABLED;
12071200
// cleanupInterningPools must be called before init(), since init() initializes a new graph,
12081201
// losing all references to the SkyKeyInterners that must be cleaned up.

‎src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,13 +362,31 @@ java_library(
362362
srcs = ["RemoteAnalysisCacheManager.java"],
363363
deps = [
364364
":analysis_cache_invalidator",
365-
":client_id",
366365
":dependencies_provider",
367366
":frontier_serializer",
368367
":options",
369368
":remote_analysis_cache_client",
370369
":remote_analysis_cache_deps",
371370
":remote_analysis_caching_server_state",
371+
"//src/main/java/com/google/devtools/build/lib/cmdline",
372+
"//src/main/java/com/google/devtools/build/lib/events",
373+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
374+
"//src/main/java/com/google/devtools/build/skyframe",
375+
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
376+
"//third_party:guava",
377+
],
378+
)
379+
380+
java_library(
381+
name = "remote_analysis_cache_factory",
382+
srcs = ["RemoteAnalysisCacheFactory.java"],
383+
deps = [
384+
":analysis_cache_invalidator",
385+
":client_id",
386+
":options",
387+
":remote_analysis_cache_client",
388+
":remote_analysis_cache_deps",
389+
":remote_analysis_cache_manager",
372390
":remote_analysis_caching_services_supplier",
373391
":remote_analysis_json_log_writer",
374392
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
@@ -396,8 +414,6 @@ java_library(
396414
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
397415
"//src/main/java/com/google/devtools/build/lib/vfs",
398416
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
399-
"//src/main/java/com/google/devtools/build/skyframe",
400-
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
401417
"//src/main/protobuf:failure_details_java_proto",
402418
"//third_party:flogger",
403419
"//third_party:gson",

‎src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCacheDeps.java‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static java.util.concurrent.TimeUnit.SECONDS;
1818

19+
import com.google.common.base.Preconditions;
1920
import com.google.common.flogger.GoogleLogger;
2021
import com.google.common.util.concurrent.ListenableFuture;
2122
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -38,7 +39,13 @@
3839
import java.util.function.Predicate;
3940
import javax.annotation.Nullable;
4041

41-
class RemoteAnalysisCacheDeps
42+
/**
43+
* Implementation of remote analysis cache functionality that is needed for reading and writing.
44+
*
45+
* <p>The parts that are needed for maintenance of Skyframe, etc. are in {@link
46+
* RemoteAnalysisCacheManager}.
47+
*/
48+
public class RemoteAnalysisCacheDeps
4249
implements SerializationDependenciesProvider, RemoteAnalysisCacheReaderDepsProvider {
4350
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
4451

@@ -60,6 +67,10 @@ class RemoteAnalysisCacheDeps
6067
private final AtomicBoolean bailedOut = new AtomicBoolean();
6168
private final ExtendedEventHandler eventHandler;
6269

70+
public static RemoteAnalysisCacheDeps createDisabled() {
71+
return new RemoteAnalysisCacheDeps();
72+
}
73+
6374
RemoteAnalysisCacheDeps(
6475
ExtendedEventHandler eventHandler,
6576
RemoteAnalysisCacheMode mode,
@@ -89,6 +100,21 @@ class RemoteAnalysisCacheDeps
89100
this.analysisCacheClient = servicesSupplier.getAnalysisCacheClient();
90101
}
91102

103+
private RemoteAnalysisCacheDeps() {
104+
this.mode = RemoteAnalysisCacheMode.OFF;
105+
this.bailOutOnMissingFingerprint = false;
106+
this.serializedFrontierProfile = "";
107+
this.activeDirectoriesMatcher = Optional.empty();
108+
this.eventHandler = null;
109+
this.jsonLogWriter = null;
110+
this.objectCodecs = null;
111+
this.listener = null;
112+
this.frontierNodeVersion = null;
113+
this.fingerprintValueServiceFuture = null;
114+
this.metadataWriter = null;
115+
this.analysisCacheClient = null;
116+
}
117+
92118
static <T> T resolveWithTimeout(Future<? extends T> future, String what)
93119
throws InterruptedException {
94120
if (future == null) {
@@ -102,28 +128,37 @@ static <T> T resolveWithTimeout(Future<? extends T> future, String what)
102128
}
103129
}
104130

131+
private void checkEnabled() {
132+
Preconditions.checkState(
133+
mode != RemoteAnalysisCacheMode.OFF, "Remote analysis cache is disabled");
134+
}
135+
105136
@Override
106137
public RemoteAnalysisCacheMode mode() {
107138
return mode;
108139
}
109140

110141
@Override
111142
public String getSerializedFrontierProfile() {
143+
checkEnabled();
112144
return serializedFrontierProfile;
113145
}
114146

115147
@Override
116148
public Optional<Predicate<PackageIdentifier>> getActiveDirectoriesMatcher() {
149+
checkEnabled();
117150
return activeDirectoriesMatcher;
118151
}
119152

120153
@Override
121154
public FrontierNodeVersion getSkyValueVersion() {
155+
checkEnabled();
122156
return frontierNodeVersion;
123157
}
124158

125159
@Override
126160
public ObjectCodecs getObjectCodecs() throws InterruptedException {
161+
checkEnabled();
127162
try {
128163
return objectCodecs.get();
129164
} catch (ExecutionException e) {
@@ -133,44 +168,52 @@ public ObjectCodecs getObjectCodecs() throws InterruptedException {
133168

134169
@Override
135170
public FingerprintValueService getFingerprintValueService() throws InterruptedException {
171+
checkEnabled();
136172
return resolveWithTimeout(fingerprintValueServiceFuture, "fingerprint value service");
137173
}
138174

139175
@Override
140176
public KeyValueWriter getFileInvalidationWriter() throws InterruptedException {
177+
checkEnabled();
141178
return getFingerprintValueService();
142179
}
143180

144181
@Override
145182
@Nullable
146183
public RemoteAnalysisCacheClient getAnalysisCacheClient() throws InterruptedException {
184+
checkEnabled();
147185
return resolveWithTimeout(analysisCacheClient, "analysis cache client");
148186
}
149187

150188
@Override
151189
@Nullable
152190
public RemoteAnalysisMetadataWriter getMetadataWriter() throws InterruptedException {
191+
checkEnabled();
153192
return resolveWithTimeout(metadataWriter, "metadata writer");
154193
}
155194

156195
@Nullable
157196
@Override
158197
public RemoteAnalysisJsonLogWriter getJsonLogWriter() {
198+
checkEnabled();
159199
return jsonLogWriter;
160200
}
161201

162202
@Override
163203
public void recordRetrievalResult(RetrievalResult retrievalResult, SkyKey key) {
204+
checkEnabled();
164205
listener.recordRetrievalResult(retrievalResult, key);
165206
}
166207

167208
@Override
168209
public void recordSerializationException(SerializationException e, SkyKey key) {
210+
checkEnabled();
169211
listener.recordSerializationException(e, key);
170212
}
171213

172214
@Override
173215
public boolean shouldBailOutOnMissingFingerprint() {
216+
checkEnabled();
174217
if (!bailOutOnMissingFingerprint) {
175218
return false;
176219
}

0 commit comments

Comments
 (0)