From c19284e3e8db5ed57097ed908c76dc902392c7ee Mon Sep 17 00:00:00 2001 From: janakr Date: Wed, 20 Jun 2018 15:22:33 -0700 Subject: Remove all uses of RuleDefinitionEnvironment#getLabel, replacing them with Label.parseAbsoluteUnchecked. Label already interns all labels, so the additional interning being done in every ConfiguredRuleClass.Builder was pointless memory and CPU. Keeping the RuleDefinitionEnvironment around makes things harder to serialize. Done using IntelliJ structural replace and then a super-painful adding of imports to every file that didn't compile (have to learn a better way to do this). PiperOrigin-RevId: 201427027 --- .../lib/analysis/ConfiguredRuleClassProvider.java | 26 +--------------------- .../lib/analysis/RuleDefinitionEnvironment.java | 10 ++------- .../lib/bazel/rules/cpp/BazelCppRuleClasses.java | 2 +- .../lib/bazel/rules/java/BazelJavaTestRule.java | 20 ++++++++--------- 4 files changed, 14 insertions(+), 44 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 0ea36aa52a..4b32382a40 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java @@ -20,9 +20,6 @@ import static com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClass import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -541,14 +538,9 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { constraintSemantics); } - @Override - public Label getLabel(String labelValue) { - return LABELS.getUnchecked(labelValue); - } - @Override public Label getToolsLabel(String labelValue) { - return getLabel(toolsRepository + labelValue); + return Label.parseAbsoluteUnchecked(toolsRepository + labelValue); } @Override @@ -565,22 +557,6 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { } } - /** - * Used to make the label instances unique, so that we don't create a new - * instance for every rule. - */ - private static final LoadingCache LABELS = CacheBuilder.newBuilder().build( - new CacheLoader() { - @Override - public Label load(String from) { - try { - return Label.parseAbsolute(from); - } catch (LabelSyntaxException e) { - throw new IllegalArgumentException(from, e); - } - } - }); - /** * Default content that should be added at the beginning of the WORKSPACE file. */ diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java index bd76287fd1..6c4705b749 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinitionEnvironment.java @@ -23,14 +23,8 @@ import javax.annotation.Nullable; */ public interface RuleDefinitionEnvironment { /** - * Parses the given string as a label and returns the label, by calling {@link - * Label#parseAbsolute}. Throws a {@link IllegalArgumentException} if the parsing fails. - */ - Label getLabel(String labelValue); - - /** - * Prepends the tools repository path to the given string and parses the result - * using {@link RuleDefinitionEnvironment#getLabel} + * Prepends the tools repository path to the given string and parses the result using {@link + * Label#parseAbsoluteUnchecked}. */ Label getToolsLabel(String labelValue); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java index 2f025439bb..a026a0a1b3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java @@ -280,7 +280,7 @@ public class BazelCppRuleClasses { // thus a dependency of def_parser. || label.startsWith(toolsRepository + "//tools/cpp") ? null - : env.getLabel(defParserLabel); + : Label.parseAbsoluteUnchecked(defParserLabel); } })); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java index dbda63a19f..776a7806c0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses.BaseJavaBinaryRule; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.TriState; @@ -55,10 +56,11 @@ public final class BazelJavaTestRule implements RuleDefinition { .setImplicitOutputsFunction(BazelJavaRuleClasses.JAVA_BINARY_IMPLICIT_OUTPUTS) // Proguard can be run over java_test targets using the --java_optimization_mode flag. // Primarily this is intended to help test changes to Proguard. - .add(attr(":proguard", LABEL) - .cfg(HostTransition.INSTANCE) - .value(JavaSemantics.PROGUARD) - .exec()) + .add( + attr(":proguard", LABEL) + .cfg(HostTransition.INSTANCE) + .value(JavaSemantics.PROGUARD) + .exec()) .add(attr(":extra_proguard_specs", LABEL_LIST).value(JavaSemantics.EXTRA_PROGUARD_SPECS)) .override(attr("stamp", TRISTATE).value(TriState.NO)) .override(attr("use_testrunner", BOOLEAN).value(true)) @@ -66,14 +68,12 @@ public final class BazelJavaTestRule implements RuleDefinition { // Input files for test actions collecting code coverage .add( attr("$lcov_merger", LABEL) - .value(env.getLabel( - "@bazel_tools//tools/test/LcovMerger/java/com/google/devtools/lcovmerger:Main" - ))) + .value( + Label.parseAbsoluteUnchecked( + "@bazel_tools//tools/test/LcovMerger/java/com/google/devtools/lcovmerger:Main"))) .add( attr("$jacocorunner", LABEL) - .value( - env.getLabel( - "@bazel_tools//tools/jdk:JacocoCoverage"))) + .value(Label.parseAbsoluteUnchecked("@bazel_tools//tools/jdk:JacocoCoverage"))) /* The Java class to be loaded by the test runner.

-- cgit v1.2.3