Skip to content

Commit 64d5bae

Browse files
joelebacopybara-github
authored andcommitted
Flip --noexperimental_enable_aggregating_middleman for Bazel.
This would disables the use of AggregatingMiddleman in Bazel. There's no expected significant performance hit, since NSOS was flipped for Bazel a while back. Benchmark (local specialist machine, runs=5): RESULTS: Bazel commit: /tmp/bazel-base, Project commit: beb859d, Project source: https://github.com/bazelbuild/bazel.git metric mean median stddev pval wall: 94.585s 94.443s 0.782s cpu: 111.896s 110.700s 3.313s system: 56.986s 55.430s 2.662s memory: 101.800MB 102.000MB 0.400MB Bazel commit: /tmp/bazel-no-mm, Project commit: beb859d, Project source: https://github.com/bazelbuild/bazel.git metric mean median stddev pval wall: 93.420s ( -1.23%) 93.617s ( -0.88%) 0.979s 0.92063 cpu: 113.764s ( +1.67%) 114.580s ( +3.50%) 3.178s 0.00000 system: 52.792s ( -7.36%) 52.450s ( -5.38%) 1.741s 0.92063 memory: 101.800MB ( +0.00%) 102.000MB ( +0.00%) 0.400MB 0.00000 RELNOTES: None PiperOrigin-RevId: 381246974
1 parent d6acb49 commit 64d5bae

File tree

6 files changed

+18
-48
lines changed

6 files changed

+18
-48
lines changed

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ public OutputPathsConverter() {
806806

807807
@Option(
808808
name = "experimental_enable_aggregating_middleman",
809-
defaultValue = "true",
809+
defaultValue = "false",
810810
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
811811
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
812812
metadataTags = {OptionMetadataTag.EXPERIMENTAL},

src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import com.google.devtools.build.lib.actions.ActionResult;
4444
import com.google.devtools.build.lib.actions.Artifact;
4545
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
46+
import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
4647
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
4748
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
4849
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
@@ -782,6 +783,15 @@ public static Artifact getFirstArtifactEndingWith(
782783
artifacts, artifact -> artifact.getExecPath().getPathString().endsWith(suffix));
783784
}
784785

786+
public static Artifact getFirstDerivedArtifactEndingWith(
787+
NestedSet<? extends Artifact> artifacts, String suffix) {
788+
return getFirstArtifactMatching(
789+
artifacts.toList(),
790+
artifact ->
791+
artifact instanceof DerivedArtifact
792+
&& artifact.getExecPath().getPathString().endsWith(suffix));
793+
}
794+
785795
/** Returns the first Artifact in the provided Iterable that matches the specified predicate. */
786796
public static Artifact getFirstArtifactMatching(
787797
Iterable<? extends Artifact> artifacts, Predicate<Artifact> predicate) {

src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSelectionTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public void testResolvedCcToolchain() throws Exception {
5757
.getToolchainContext()
5858
.forToolchainType(Label.parseAbsolute(CPP_TOOLCHAIN_TYPE, ImmutableMap.of()));
5959
CcToolchainProvider toolchain = (CcToolchainProvider) toolchainInfo.getValue("cc");
60-
assertThat(toolchain.getCompilerFiles().getSingleton().getExecPathString()).endsWith("k8");
60+
assertThat(toolchain.getToolchainIdentifier()).endsWith("k8");
6161
}
6262

6363
@Test

src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,12 @@
1515
package com.google.devtools.build.lib.rules.cpp;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18-
import static com.google.common.truth.Truth.assertWithMessage;
1918
import static org.junit.Assert.assertThrows;
2019

2120
import com.google.common.base.Joiner;
2221
import com.google.common.collect.ImmutableList;
2322
import com.google.common.collect.ImmutableSet;
2423
import com.google.common.collect.MoreCollectors;
25-
import com.google.common.collect.Sets;
2624
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
2725
import com.google.devtools.build.lib.actions.Artifact;
2826
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -39,7 +37,6 @@
3937
import com.google.devtools.build.lib.testutil.TestConstants;
4038
import com.google.devtools.common.options.OptionsParsingException;
4139
import java.io.IOException;
42-
import java.util.Set;
4340
import org.junit.Test;
4441
import org.junit.runner.RunWith;
4542
import org.junit.runners.JUnit4;
@@ -676,37 +673,6 @@ public void correctToolFilesUsed() throws Exception {
676673
useConfiguration("--incompatible_use_specific_tool_files");
677674
ConfiguredTarget target = getConfiguredTarget("//a:a");
678675
CcToolchainProvider toolchainProvider = target.get(CcToolchainProvider.PROVIDER);
679-
// Check that the mock toolchain tool file sets are an antichain, so that our subset assertions
680-
// below are meaningful.
681-
ImmutableList<Set<Artifact>> fileGroups =
682-
ImmutableList.of(
683-
toolchainProvider.getArFiles().toSet(),
684-
toolchainProvider.getLinkerFiles().toSet(),
685-
toolchainProvider.getCompilerFiles().toSet(),
686-
toolchainProvider.getAsFiles().toSet(),
687-
toolchainProvider.getAllFiles().toSet());
688-
for (int i = 0; i < fileGroups.size(); i++) {
689-
assertThat(fileGroups.get(i)).isNotEmpty();
690-
for (int j = 0; j < fileGroups.size(); j++) {
691-
if (i == j) {
692-
continue;
693-
}
694-
Set<Artifact> one = fileGroups.get(i);
695-
Set<Artifact> two = fileGroups.get(j);
696-
assertWithMessage(String.format("%s should not contain %s", one, two))
697-
.that(one.containsAll(two))
698-
.isFalse();
699-
}
700-
}
701-
assertThat(
702-
Sets.difference(
703-
toolchainProvider.getArFiles().toSet(), toolchainProvider.getLinkerFiles().toSet()))
704-
.isNotEmpty();
705-
assertThat(
706-
Sets.difference(
707-
toolchainProvider.getLinkerFiles().toSet(), toolchainProvider.getArFiles().toSet()))
708-
.isNotEmpty();
709-
710676
RuleConfiguredTarget libTarget = (RuleConfiguredTarget) getConfiguredTarget("//a:l");
711677
Artifact staticLib =
712678
getOutputGroup(libTarget, "archive").toList().stream()

src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,6 @@ public class ObjcLibraryTest extends ObjcRuleTestCase {
7474

7575
static final RuleType RULE_TYPE = new OnlyNeedsSourcesRuleType("objc_library");
7676
private static final String WRAPPED_CLANG = "wrapped_clang";
77-
78-
/**
79-
* Middleman artifact arising from //tools/osx/crosstool:link, containing tools that should be
80-
* inputs to link actions.
81-
*/
82-
private static final String CROSSTOOL_LINK_MIDDLEMAN = "tools_Sosx_Scrosstool_Clink";
83-
8477
/** Creates an {@code objc_library} target writer. */
8578
@Override
8679
protected ScratchAttributeWriter createLibraryTargetWriter(String labelString) {
@@ -878,7 +871,7 @@ public void testArchiveAction_simulator() throws Exception {
878871
"-o",
879872
Iterables.getOnlyElement(archiveAction.getOutputs()).getExecPathString()));
880873
assertThat(baseArtifactNames(archiveAction.getInputs()))
881-
.containsAtLeast("a.o", "b.o", "lib-archive.objlist", CROSSTOOL_LINK_MIDDLEMAN);
874+
.containsAtLeast("a.o", "b.o", "lib-archive.objlist", "ar", "libempty.a", "libtool");
882875
assertThat(baseArtifactNames(archiveAction.getOutputs())).containsExactly("liblib.a");
883876
assertRequiresDarwin(archiveAction);
884877
}

src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.common.truth.Truth.assertThat;
1919
import static com.google.common.truth.Truth8.assertThat;
2020
import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
21+
import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstDerivedArtifactEndingWith;
2122
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.MODULE_MAP;
2223
import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.LIPO;
2324
import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.SRCS_TYPE;
@@ -1407,9 +1408,9 @@ public void checkLinkingRuleCanUseCrosstool_singleArch(RuleType ruleType) throws
14071408

14081409
// If bin is indeed using the c++ backend, then its archive action should be a CppLinkAction.
14091410
Action lipobinAction = lipoBinAction("//x:x");
1410-
Artifact bin = getFirstArtifactEndingWith(lipobinAction.getInputs(), "_bin");
1411+
Artifact bin = getFirstDerivedArtifactEndingWith(lipobinAction.getInputs(), "_bin");
14111412
Action linkAction = getGeneratingAction(bin);
1412-
Artifact archive = getFirstArtifactEndingWith(linkAction.getInputs(), ".a");
1413+
Artifact archive = getFirstDerivedArtifactEndingWith(linkAction.getInputs(), ".a");
14131414
Action archiveAction = getGeneratingAction(archive);
14141415
assertThat(archiveAction).isInstanceOf(CppLinkAction.class);
14151416
}
@@ -1420,9 +1421,9 @@ public void checkLinkingRuleCanUseCrosstool_multiArch(RuleType ruleType) throws
14201421

14211422
// If bin is indeed using the c++ backend, then its archive action should be a CppLinkAction.
14221423
Action lipobinAction = lipoBinAction("//x:x");
1423-
Artifact bin = getFirstArtifactEndingWith(lipobinAction.getInputs(), "_bin");
1424+
Artifact bin = getFirstDerivedArtifactEndingWith(lipobinAction.getInputs(), "_bin");
14241425
Action linkAction = getGeneratingAction(bin);
1425-
Artifact archive = getFirstArtifactEndingWith(linkAction.getInputs(), ".a");
1426+
Artifact archive = getFirstDerivedArtifactEndingWith(linkAction.getInputs(), ".a");
14261427
Action archiveAction = getGeneratingAction(archive);
14271428
assertThat(archiveAction).isInstanceOf(CppLinkAction.class);
14281429
}

0 commit comments

Comments
 (0)