Skip to content

Commit

Permalink
perf: Pass --override_repository flag to all commands for faster anal…
Browse files Browse the repository at this point in the history
…ysis phase (#6453)

* Pass --override_repository flag to all commands run from the plugin.

Using this flag ensures consistency across commands and prevents large portions
of the Skyframe cache from being evicted. This avoids longer analysis phases.
To experiment, view the cache summary with `bazel dump --skyframe=summary`.

* Fix android studio tests

* Fix AS tests

* Missing semicolon

* Remove unused method "getAspectFlags()"

* Convert to optional
  • Loading branch information
tpasternak authored May 31, 2024
1 parent 932b3f7 commit cde116d
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static void main(String[] a) throws Exception {
// Flags for wiring up the plugin aspect from the external @intellij_aspect repository.
ImmutableList<String> aspectFlags =
ImmutableList.of(
aspectStrategyBazel.getAspectFlag(),
aspectStrategyBazel.getAspectFlag().get(),
String.format(
"%s=%s/%s/aspect",
AspectStrategyBazel.OVERRIDE_REPOSITORY_FLAG,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.idea.common.experiments.MockExperimentService;
import com.intellij.openapi.extensions.impl.ExtensionPointImpl;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -137,8 +138,8 @@ public String getName() {
}

@Override
protected List<String> getAspectFlags() {
return ImmutableList.of();
protected Optional<String> getAspectFlag() {
return Optional.empty();
}
}
}
5 changes: 5 additions & 0 deletions base/src/com/google/idea/blaze/base/command/BlazeCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.idea.blaze.base.bazel.BuildSystem.BuildInvoker;
import com.google.idea.blaze.base.model.primitives.TargetExpression;
import com.google.idea.blaze.base.sync.aspects.strategy.AspectStrategyBazel;

import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -108,6 +110,9 @@ public Builder(String binaryPath, BlazeCommandName name) {
this.invokeParallel = false;
// Tell forge what tool we used to call blaze so we can track usage.
addBlazeFlags(BlazeFlags.getToolTagFlag());
AspectStrategyBazel.getAspectRepositoryOverrideFlag().ifPresent(it ->
addBlazeFlags(it)
);
}

private ImmutableList<String> getArguments() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Predicate;
Expand Down Expand Up @@ -88,7 +89,7 @@ protected AspectStrategy(boolean aspectSupportsDirectDepsTrimming) {

public abstract String getName();

protected abstract List<String> getAspectFlags();
protected abstract Optional<String> getAspectFlag();

/**
* Add the aspect to the build and request the given {@code OutputGroup}s. This method should only
Expand All @@ -107,7 +108,7 @@ public final void addAspectAndOutputGroups(
.flatMap(g -> getOutputGroups(g, activeLanguages, directDepsOnly).stream())
.collect(toImmutableList());
builder
.addBlazeFlags(getAspectFlags())
.addBlazeFlags(getAspectFlag().map(List::of).orElse(List.of()))
.addBlazeFlags("--output_groups=" + Joiner.on(',').join(groups));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@
package com.google.idea.blaze.base.sync.aspects.strategy;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.idea.blaze.base.model.BlazeVersionData;
import com.google.idea.blaze.base.settings.BuildSystemName;
import com.intellij.ide.plugins.IdeaPluginDescriptor;
import com.intellij.ide.plugins.PluginManager;
import java.io.File;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;

/** Aspect strategy for Bazel, where the aspect is situated in an external repository. */
Expand All @@ -49,9 +48,10 @@ public AspectStrategyBazel(BlazeVersionData versionData) {
}
}

@Override
@VisibleForTesting
public String getAspectFlag() {
return aspectFlag;
public Optional<String> getAspectFlag() {
return Optional.of(aspectFlag);
}

// In tests, the location of @intellij_aspect is not known at compile time.
Expand All @@ -62,18 +62,16 @@ public String getName() {
return "AspectStrategySkylarkBazel";
}

@Override
protected List<String> getAspectFlags() {
return ImmutableList.of(aspectFlag, getAspectRepositoryOverrideFlag());
}

private static File findAspectDirectory() {
private static Optional<File> findAspectDirectory() {
IdeaPluginDescriptor plugin =
PluginManager.getPlugin(PluginManager.getPluginByClassName(AspectStrategy.class.getName()));
return new File(plugin.getPath(), "aspect");
if (plugin == null) {
return Optional.empty();
}
return Optional.of(new File(plugin.getPath(), "aspect"));
}

private static String getAspectRepositoryOverrideFlag() {
return OVERRIDE_REPOSITORY_FLAG + "=" + findAspectDirectory().getPath();
public static Optional<String> getAspectRepositoryOverrideFlag() {
return findAspectDirectory().map(it -> OVERRIDE_REPOSITORY_FLAG + "=" + it.getPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@
import com.google.idea.common.experiments.MockExperimentService;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;


/** Unit tests for {@link AspectStrategy}. */
@RunWith(JUnit4.class)
public class AspectStrategyTest extends BlazeTestCase {
Expand Down Expand Up @@ -212,8 +214,8 @@ public String getName() {
}

@Override
protected List<String> getAspectFlags() {
return ImmutableList.of();
protected Optional<String> getAspectFlag() {
return Optional.empty();
}
}
}

0 comments on commit cde116d

Please sign in to comment.