From 9d6681a5bb991352e18dc6b6139f0d1e7039099b Mon Sep 17 00:00:00 2001 From: Daniel Brauner <44034965+LeFrosch@users.noreply.github.com> Date: Sat, 23 Nov 2024 00:20:47 +0100 Subject: [PATCH] Use BuildArtifactCache to fetch -info.txt files (#7024) Fetching multiple info files sequentially is slow especially whey they are enormous in size. (The total size reaching 6gb+). Instead fetch artifacts through the `BuildArtifactCache` which already handles parallel and asynchronous downloading well. Also, remove the `qsync.parallel.artifact.info.fetch` experiment as it not longer controls anything. Info files are not different to other artifacts and thus the same mode that works for other artifacts should work for info files. Bug: n/a Test: n/a Change-Id: I6f023154ca318af390956e78678dc899a359d31d AOSP: 9c2f4c7143874defeb9cb9c69f69168975f38d5b Co-authored-by: Googler --- .../base/qsync/BazelDependencyBuilder.java | 133 +++++++++++------- .../idea/blaze/base/qsync/ProjectLoader.java | 11 +- 2 files changed, 90 insertions(+), 54 deletions(-) diff --git a/base/src/com/google/idea/blaze/base/qsync/BazelDependencyBuilder.java b/base/src/com/google/idea/blaze/base/qsync/BazelDependencyBuilder.java index 186fb23e61a..2c5ae202d40 100644 --- a/base/src/com/google/idea/blaze/base/qsync/BazelDependencyBuilder.java +++ b/base/src/com/google/idea/blaze/base/qsync/BazelDependencyBuilder.java @@ -19,10 +19,13 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.util.concurrent.Futures.immediateFailedFuture; +import static com.google.common.util.concurrent.Futures.immediateFuture; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.joining; import com.google.common.base.Joiner; +import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; @@ -31,7 +34,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.common.io.ByteSource; -import com.google.common.io.CharSource; import com.google.common.io.MoreFiles; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; @@ -54,15 +56,18 @@ import com.google.idea.blaze.base.scope.BlazeContext; import com.google.idea.blaze.base.sync.aspects.BlazeBuildOutputs; import com.google.idea.blaze.base.vcs.BlazeVcsHandlerProvider.BlazeVcsHandler; +import com.google.idea.blaze.common.Context; import com.google.idea.blaze.common.Label; +import com.google.idea.blaze.common.Output; import com.google.idea.blaze.common.PrintOutput; import com.google.idea.blaze.common.artifact.BlazeArtifact; +import com.google.idea.blaze.common.artifact.BuildArtifactCache; +import com.google.idea.blaze.common.artifact.CachedArtifact; import com.google.idea.blaze.common.artifact.OutputArtifact; import com.google.idea.blaze.common.proto.ProtoStringInterner; import com.google.idea.blaze.common.vcs.VcsState; import com.google.idea.blaze.exception.BuildException; import com.google.idea.blaze.qsync.BlazeQueryParser; -import com.google.idea.blaze.qsync.artifacts.ArtifactDirectoryUpdate; import com.google.idea.blaze.qsync.deps.DependencyBuildContext; import com.google.idea.blaze.qsync.deps.OutputGroup; import com.google.idea.blaze.qsync.deps.OutputInfo; @@ -80,7 +85,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.nio.file.CopyOption; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -92,13 +96,11 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; +import java.util.function.Function; /** An object that knows how to build dependencies for given targets */ public class BazelDependencyBuilder implements DependencyBuilder { - public static final BoolExperiment fetchArtifactInfoInParallel = - new BoolExperiment("qsync.parallel.artifact.info.fetch", true); - public static final BoolExperiment buildGeneratedSrcJars = new BoolExperiment("qsync.build.generated.src.jars", true); @@ -120,20 +122,23 @@ public class BazelDependencyBuilder implements DependencyBuilder { protected final WorkspaceRoot workspaceRoot; protected final Optional vcsHandler; protected final ImmutableSet handledRuleKinds; + protected final BuildArtifactCache buildArtifactCache; public BazelDependencyBuilder( - Project project, - BuildSystem buildSystem, - ProjectDefinition projectDefinition, - WorkspaceRoot workspaceRoot, - Optional vcsHandler, - ImmutableSet handledRuleKinds) { + Project project, + BuildSystem buildSystem, + ProjectDefinition projectDefinition, + WorkspaceRoot workspaceRoot, + Optional vcsHandler, + BuildArtifactCache buildArtifactCache, + ImmutableSet handledRuleKinds) { this.project = project; this.buildSystem = buildSystem; this.projectDefinition = projectDefinition; this.workspaceRoot = workspaceRoot; this.vcsHandler = vcsHandler; this.handledRuleKinds = handledRuleKinds; + this.buildArtifactCache = buildArtifactCache; } private static final ImmutableMultimap OUTPUT_GROUPS_BY_LANGUAGE = @@ -301,40 +306,27 @@ private OutputInfo createOutputInfo( || totalBytesToFetch > FETCH_SIZE_LOG_THRESHOLD; if (shouldLog) { context.output( - PrintOutput.log( - String.format( - "Fetching %d artifact info files (%s)", - totalFilesToFetch, StringUtilRt.formatFileSize(totalBytesToFetch)))); + PrintOutput.log( + String.format( + "Fetching and parsing %d artifact info files (%s)", + totalFilesToFetch, StringUtilRt.formatFileSize(totalBytesToFetch)))); } ImmutableSet.Builder artifactInfoFilesBuilder = ImmutableSet.builder(); ImmutableSet.Builder ccInfoBuilder = ImmutableSet.builder(); - if (fetchArtifactInfoInParallel.getValue()) { - try { - ListenableFuture> artifactInfoFutures = - readAndTransformInfoFiles(artifactInfoFiles, this::readArtifactInfoFile); - ListenableFuture> ccInfoFutures = - readAndTransformInfoFiles(ccArtifactInfoFiles, this::readCcInfoFile); - - artifactInfoFilesBuilder.addAll(Uninterruptibles.getUninterruptibly(artifactInfoFutures)); - ccInfoBuilder.addAll(Uninterruptibles.getUninterruptibly(ccInfoFutures)); - } catch (ExecutionException e) { - throw new BuildException(e); - } - } else { - for (OutputArtifact artifactInfoFile : artifactInfoFiles) { - artifactInfoFilesBuilder.add(readArtifactInfoFile(artifactInfoFile)); - } - for (OutputArtifact artifactInfoFile : ccArtifactInfoFiles) { - ccInfoBuilder.add(readCcInfoFile(artifactInfoFile)); - } - } + List artifactInfos = + readAndTransformInfoFiles(context, artifactInfoFiles, this::readArtifactInfoFile); + List ccInfos = + readAndTransformInfoFiles(context, ccArtifactInfoFiles, this::readCcInfoFile); + + artifactInfoFilesBuilder.addAll(artifactInfos); + ccInfoBuilder.addAll(ccInfos); long elapsed = System.currentTimeMillis() - startTime; if (shouldLog) { context.output( - PrintOutput.log(String.format("Fetched artifact info files in %d ms", elapsed))); + PrintOutput.log(String.format("Fetched and parsed artifact info files in %d ms", elapsed))); } Optional vcsState = Optional.empty(); if (blazeBuildOutputs.sourceUri.isPresent() && vcsHandler.isPresent()) { @@ -366,29 +358,72 @@ private interface CheckedTransform { R apply(T t) throws BuildException; } - private ListenableFuture> readAndTransformInfoFiles( - ImmutableList artifactInfoFiles, - CheckedTransform transform) { - List> futures = Lists.newArrayList(); - for (OutputArtifact artifactInfoFile : artifactInfoFiles) { - futures.add(Futures.submit(() -> transform.apply(artifactInfoFile), FetchExecutor.EXECUTOR)); + + private ImmutableList readAndTransformInfoFiles( + Context context, + ImmutableList artifactInfoFiles, + CheckedTransform transform) throws BuildException { + + ListenableFuture listenableFuture = buildArtifactCache.addAll(artifactInfoFiles, context); + Stopwatch sw = Stopwatch.createStarted(); + // TODO: solodkyy - For now separate fetching and parsing. We have had some thread safety issues and it allows us reporting fetching + // time correctly which happens to be much shorter than the time it takes to parse the info files. + try { + final var unused = listenableFuture.get(); + } + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new BuildException(e); + } + catch (ExecutionException e) { + throw new BuildException(e); + } + context.output(PrintOutput.output("Fetched %d info files in %sms", artifactInfoFiles.size(), sw.elapsed().toMillis())); + ImmutableList> artifactFutures = + artifactInfoFiles.stream() + .map(OutputArtifact::getDigest) + .map(digest -> { + final var result = buildArtifactCache.get(digest); + if (result.isEmpty()) { + context.output(PrintOutput.error("Failed to get artifact future for: " + digest)); + context.setHasError(); + } + return result; + }) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(toImmutableList()); + + ImmutableList> futures = + artifactFutures.stream() + .map(it -> + Futures.transformAsync(it, a -> immediateFuture(transform.apply(a.byteSource())), FetchExecutor.EXECUTOR)) + .collect(toImmutableList()); + try { + return ImmutableList.copyOf(Futures.allAsList(futures).get()); + } + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new BuildException(e); + } + catch (ExecutionException e) { + throw new BuildException(e); } - return Futures.allAsList(futures); } - private JavaArtifacts readArtifactInfoFile(BlazeArtifact file) throws BuildException { + private JavaArtifacts readArtifactInfoFile(ByteSource file) throws BuildException { return ProtoStringInterner.intern( readArtifactInfoProtoFile(JavaArtifacts.newBuilder(), file).build()); } - private CcCompilationInfo readCcInfoFile(BlazeArtifact file) throws BuildException { + private CcCompilationInfo readCcInfoFile(ByteSource file) throws BuildException { return ProtoStringInterner.intern( readArtifactInfoProtoFile(CcCompilationInfo.newBuilder(), file).build()); } - protected B readArtifactInfoProtoFile(B builder, BlazeArtifact file) - throws BuildException { - try (InputStream inputStream = file.getInputStream()) { + protected B readArtifactInfoProtoFile(B builder, ByteSource file) + throws BuildException { + try (InputStream inputStream = file.openStream()) { TextFormat.Parser parser = TextFormat.Parser.newBuilder().build(); parser.merge(new InputStreamReader(inputStream, UTF_8), builder); return builder; diff --git a/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java b/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java index b4d004245c3..e50813fa3c1 100644 --- a/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java +++ b/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java @@ -62,7 +62,6 @@ import com.google.idea.blaze.qsync.java.ParallelPackageReader; import com.google.idea.blaze.qsync.project.ProjectDefinition; import com.google.idea.blaze.qsync.project.ProjectPath; -import com.google.idea.blaze.qsync.query.QuerySpec; import com.google.idea.blaze.qsync.query.QuerySpec.QueryStrategy; import com.google.idea.common.experiments.BoolExperiment; import com.intellij.openapi.project.Project; @@ -139,9 +138,6 @@ public QuerySyncProject loadProject(BlazeContext context) throws BuildException ImmutableSet handledRules = getHandledRuleKinds(); Optional vcsHandler = Optional.ofNullable(BlazeVcsHandlerProvider.vcsHandlerForProject(project)); - DependencyBuilder dependencyBuilder = - createDependencyBuilder( - workspaceRoot, latestProjectDef, buildSystem, vcsHandler, handledRules); RenderJarBuilder renderJarBuilder = createRenderJarBuilder(workspaceRoot, buildSystem); AppInspectorBuilder appInspectorBuilder = createAppInspectorBuilder(buildSystem); @@ -159,6 +155,10 @@ public QuerySyncProject loadProject(BlazeContext context) throws BuildException executor, QuerySyncManager.getInstance(project).cacheCleanRequest()); + DependencyBuilder dependencyBuilder = + createDependencyBuilder( + workspaceRoot, latestProjectDef, buildSystem, vcsHandler, artifactCache, handledRules); + ArtifactTracker artifactTracker; RenderJarArtifactTracker renderJarArtifactTracker; AppInspectorArtifactTracker appInspectorArtifactTracker; @@ -273,9 +273,10 @@ protected DependencyBuilder createDependencyBuilder( ProjectDefinition projectDefinition, BuildSystem buildSystem, Optional vcsHandler, + BuildArtifactCache buildArtifactCache, ImmutableSet handledRuleKinds) { return new BazelDependencyBuilder( - project, buildSystem, projectDefinition, workspaceRoot, vcsHandler, handledRuleKinds); + project, buildSystem, projectDefinition, workspaceRoot, vcsHandler, buildArtifactCache, handledRuleKinds); } protected RenderJarBuilder createRenderJarBuilder(