From 9ed6ebbcb2731c25749b18d20c9a44c9a284bbb8 Mon Sep 17 00:00:00 2001 From: idanakav Date: Tue, 8 Aug 2023 11:54:35 -0700 Subject: [PATCH 1/2] Skip computing working set when possible Computing the working set involves running few git commands, including git diff, it can be skipped if the "Expand Sync to Working Set" option is disabled. The working set will be computed on each `collectProjectState` call, given that this method is triggered three times per sync operation, disabling unnecessary working set computations can reduce the sync times roughly the amount of time taken by 3 git diff runs. --- .../idea/blaze/base/sync/BlazeSyncManager.java | 2 +- .../idea/blaze/base/sync/ProjectStateSyncTask.java | 14 ++++++++++---- .../idea/blaze/base/sync/SyncPhaseCoordinator.java | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/base/src/com/google/idea/blaze/base/sync/BlazeSyncManager.java b/base/src/com/google/idea/blaze/base/sync/BlazeSyncManager.java index defe1b0ae64..6a342718a71 100644 --- a/base/src/com/google/idea/blaze/base/sync/BlazeSyncManager.java +++ b/base/src/com/google/idea/blaze/base/sync/BlazeSyncManager.java @@ -156,7 +156,7 @@ public void requestProjectSync(BlazeSyncParams syncParams) { try { projectState = ProjectStateSyncTask.collectProjectState( - project, context); + project, context, syncParams); } catch (SyncCanceledException | SyncFailedException e) { ApplicationManager.getApplication() .invokeLater( diff --git a/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java b/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java index cb276dfc243..011e8cdebe6 100644 --- a/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java +++ b/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java @@ -17,6 +17,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.idea.blaze.base.async.FutureUtil; @@ -61,10 +62,10 @@ /** Collects information about the project state (VCS, blaze info, .blazeproject contents, etc.). */ final class ProjectStateSyncTask { - static SyncProjectState collectProjectState(Project project, BlazeContext context) + static SyncProjectState collectProjectState(Project project, BlazeContext context, BlazeSyncParams syncParams) throws SyncCanceledException, SyncFailedException { ProjectStateSyncTask task = new ProjectStateSyncTask(project); - return task.getProjectState(context); + return task.getProjectState(context, syncParams); } private final Project project; @@ -77,7 +78,7 @@ private ProjectStateSyncTask(Project project) { this.workspaceRoot = WorkspaceRoot.fromImportSettings(importSettings); } - private SyncProjectState getProjectState(BlazeContext context) + private SyncProjectState getProjectState(BlazeContext context, BlazeSyncParams params) throws SyncFailedException, SyncCanceledException { if (!FileOperationProvider.getInstance().exists(workspaceRoot.directory())) { String message = String.format("Workspace '%s' doesn't exist.", workspaceRoot.directory()); @@ -124,7 +125,12 @@ private SyncProjectState getProjectState(BlazeContext context) importSettings.getBuildSystem(), syncFlags); - ListenableFuture workingSetFuture = vcsHandler.getWorkingSet(context, executor); + ListenableFuture workingSetFuture; + if(params.addWorkingSet() || params.syncMode() == SyncMode.FULL) { + workingSetFuture = vcsHandler.getWorkingSet(context, executor); + } else { + workingSetFuture = Futures.immediateFuture(null); + } BlazeInfo blazeInfo = FutureUtil.waitForFuture(context, blazeInfoFuture) diff --git a/base/src/com/google/idea/blaze/base/sync/SyncPhaseCoordinator.java b/base/src/com/google/idea/blaze/base/sync/SyncPhaseCoordinator.java index d96202016f2..632d6d70a3b 100644 --- a/base/src/com/google/idea/blaze/base/sync/SyncPhaseCoordinator.java +++ b/base/src/com/google/idea/blaze/base/sync/SyncPhaseCoordinator.java @@ -361,7 +361,7 @@ private void doFilterProjectTargets( context, childContext -> { SyncProjectState projectState = - ProjectStateSyncTask.collectProjectState(project, context); + ProjectStateSyncTask.collectProjectState(project, context, params); if (projectState == null) { return; } @@ -436,7 +436,7 @@ void runSync(BlazeSyncParams params, boolean singleThreaded, BlazeContext contex SyncStats.builder()); return; } - SyncProjectState projectState = ProjectStateSyncTask.collectProjectState(project, context); + SyncProjectState projectState = ProjectStateSyncTask.collectProjectState(project, context, params); BlazeSyncBuildResult buildResult = BuildPhaseSyncTask.runBuildPhase( project, params, projectState, buildId, context, buildSystem); From c12b09defb0671c985e682ebd039e642e268d449 Mon Sep 17 00:00:00 2001 From: idanakav Date: Tue, 8 Aug 2023 15:25:08 -0700 Subject: [PATCH 2/2] Cache blaze info during sync Currently, BlazeInfo will be computed 5 times during incremental sync, this commit introduces logic to cache it and re-use the computed value during incremental syncs. `BlazeInfoProvider` was introduced in order to avoid changing `BlazeInfoRunner` and keep its responsibility to solely execute the info command. Logic is guarded by `blaze.info.provider.enabled` which is disabled by default. --- base/src/META-INF/blaze-base.xml | 2 + .../base/bazel/AbstractBuildInvoker.java | 11 +- .../base/command/info/BlazeInfoProvider.java | 127 ++++++++++++++++++ .../blaze/base/sync/ProjectStateSyncTask.java | 34 +++-- 4 files changed, 164 insertions(+), 10 deletions(-) create mode 100644 base/src/com/google/idea/blaze/base/command/info/BlazeInfoProvider.java diff --git a/base/src/META-INF/blaze-base.xml b/base/src/META-INF/blaze-base.xml index d87ed6b2b86..2ce34b9dd66 100644 --- a/base/src/META-INF/blaze-base.xml +++ b/base/src/META-INF/blaze-base.xml @@ -226,6 +226,7 @@ + @@ -535,6 +536,7 @@ + diff --git a/base/src/com/google/idea/blaze/base/bazel/AbstractBuildInvoker.java b/base/src/com/google/idea/blaze/base/bazel/AbstractBuildInvoker.java index 003993b1e32..5e76138a86c 100644 --- a/base/src/com/google/idea/blaze/base/bazel/AbstractBuildInvoker.java +++ b/base/src/com/google/idea/blaze/base/bazel/AbstractBuildInvoker.java @@ -28,6 +28,7 @@ import com.google.idea.blaze.base.command.buildresult.BuildResultHelper; import com.google.idea.blaze.base.command.buildresult.BuildResultHelperBep; import com.google.idea.blaze.base.command.info.BlazeInfo; +import com.google.idea.blaze.base.command.info.BlazeInfoProvider; import com.google.idea.blaze.base.command.info.BlazeInfoRunner; import com.google.idea.blaze.base.projectview.ProjectViewManager; import com.google.idea.blaze.base.projectview.ProjectViewSet; @@ -144,7 +145,15 @@ private ListenableFuture runBlazeInfo() { BlazeCommandName.INFO, blazeContext, BlazeInvocationContext.SYNC_CONTEXT); + if (BlazeInfoProvider.isEnabled()) { + return BlazeInfoProvider.getInstance(project) + .getBlazeInfo(blazeContext, syncFlags); + } return BlazeInfoRunner.getInstance() - .runBlazeInfo(project, this, blazeContext, buildSystem.getName(), syncFlags); + .runBlazeInfo(project, + this, + blazeContext, + buildSystem.getName(), + syncFlags); } } diff --git a/base/src/com/google/idea/blaze/base/command/info/BlazeInfoProvider.java b/base/src/com/google/idea/blaze/base/command/info/BlazeInfoProvider.java new file mode 100644 index 00000000000..ab8ca6c6787 --- /dev/null +++ b/base/src/com/google/idea/blaze/base/command/info/BlazeInfoProvider.java @@ -0,0 +1,127 @@ +/* + * Copyright 2023 The Bazel Authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.idea.blaze.base.command.info; + +import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.idea.blaze.base.async.executor.BlazeExecutor; +import com.google.idea.blaze.base.bazel.BuildSystem; +import com.google.idea.blaze.base.scope.BlazeContext; +import com.google.idea.blaze.base.settings.Blaze; +import com.google.idea.blaze.base.settings.BlazeImportSettings; +import com.google.idea.blaze.base.settings.BlazeImportSettingsManager; +import com.google.idea.blaze.base.sync.SyncListener; +import com.google.idea.blaze.base.sync.SyncMode; +import com.google.idea.blaze.base.sync.SyncResult; +import com.google.idea.common.experiments.BoolExperiment; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.project.Project; +import org.jetbrains.annotations.Nullable; + +import java.util.List; +import java.util.concurrent.ExecutionException; + +/** + *

Providing a {@link com.google.idea.blaze.base.command.info.BlazeInfo}, the result will be cached and invalidated on any sync failure.

+ * + * @see BlazeInfoRunner + */ +public class BlazeInfoProvider { + private static final Logger logger = Logger.getInstance(BlazeInfoProvider.class); + private static final BoolExperiment enabled = + new BoolExperiment("blaze.info.provider.enabled", false); + + private final Project project; + private volatile BlazeInfo blazeInfo; + + public BlazeInfoProvider(Project project) { + this.project = project; + } + + public static BlazeInfoProvider getInstance(Project project) { + return project.getService(BlazeInfoProvider.class); + } + + public static Boolean isEnabled() { + return enabled.getValue(); + } + + public ListenableFuture getBlazeInfo( + BlazeContext context, + List blazeFlags) { + return BlazeExecutor.getInstance().submit(() -> { + BlazeInfo info = getCachedBlazeInfo(context, + blazeFlags); + if (info == null) { + throw new BlazeInfoException("Unable to get blaze info"); + } + return info; + }); + } + + private @Nullable BlazeInfo getCachedBlazeInfo( + BlazeContext context, + List blazeFlags) { + if (blazeInfo != null) { + logger.debug("Returning cached BlazeInfo"); + return blazeInfo; + } + try { + BlazeImportSettings importSettings = BlazeImportSettingsManager.getInstance(project).getImportSettings(); + if (importSettings == null) { + return null; + } + BuildSystem.BuildInvoker buildInvoker = Blaze.getBuildSystemProvider(project) + .getBuildSystem() + .getDefaultInvoker(project, context); + logger.debug("Running bazel info"); + blazeInfo = BlazeInfoRunner.getInstance().runBlazeInfo( + project, + buildInvoker, + BlazeContext.create(), + importSettings.getBuildSystem(), + blazeFlags + ).get(); + return blazeInfo; + } catch (InterruptedException | ExecutionException e) { + logger.warn("Unable to run blaze info", e); + return null; + } + } + + public void invalidate() { + logger.debug("invalidating"); + blazeInfo = null; + } + + public static final class Invalidator implements SyncListener { + @Override + public void afterSync( + Project project, + BlazeContext context, + SyncMode syncMode, + SyncResult syncResult, + ImmutableSet buildIds) { + if(!isEnabled()) { + return; + } + BlazeInfoProvider provider = BlazeInfoProvider.getInstance(project); + if(provider != null) { + provider.invalidate(); + } + } + } +} diff --git a/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java b/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java index 011e8cdebe6..14dbb8ddab6 100644 --- a/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java +++ b/base/src/com/google/idea/blaze/base/sync/ProjectStateSyncTask.java @@ -26,6 +26,7 @@ import com.google.idea.blaze.base.command.BlazeFlags; import com.google.idea.blaze.base.command.BlazeInvocationContext; import com.google.idea.blaze.base.command.info.BlazeInfo; +import com.google.idea.blaze.base.command.info.BlazeInfoProvider; import com.google.idea.blaze.base.command.info.BlazeInfoRunner; import com.google.idea.blaze.base.io.FileOperationProvider; import com.google.idea.blaze.base.model.BlazeVersionData; @@ -115,15 +116,7 @@ private SyncProjectState getProjectState(BlazeContext context, BlazeSyncParams p BlazeInvocationContext.SYNC_CONTEXT); ListenableFuture blazeInfoFuture = - BlazeInfoRunner.getInstance() - .runBlazeInfo( - project, - Blaze.getBuildSystemProvider(project) - .getBuildSystem() - .getDefaultInvoker(project, context), - context, - importSettings.getBuildSystem(), - syncFlags); + createBazelInfoFuture(context, syncFlags, params.syncMode()); ListenableFuture workingSetFuture; if(params.addWorkingSet() || params.syncMode() == SyncMode.FULL) { @@ -189,6 +182,29 @@ private SyncProjectState getProjectState(BlazeContext context, BlazeSyncParams p .build(); } + private ListenableFuture createBazelInfoFuture( + BlazeContext context, + List syncFlags, + SyncMode syncMode) { + boolean useBazelInfoRunner = !BlazeInfoProvider.isEnabled() || syncMode == SyncMode.FULL; + if (useBazelInfoRunner) { + return BlazeInfoRunner.getInstance() + .runBlazeInfo( + project, + Blaze.getBuildSystemProvider(project) + .getBuildSystem() + .getDefaultInvoker(project, + context), + context, + importSettings.getBuildSystem(), + syncFlags); + } + return BlazeInfoProvider.getInstance(project) + .getBlazeInfo( + context, + syncFlags); + } + private static class WorkspacePathResolverAndProjectView { final WorkspacePathResolver workspacePathResolver; final ProjectViewSet projectViewSet;