Replace @BlazeRule with a getMetadata() method. This lets us avoid Java Reflection, and saves ~15% of initialization time in []RuleClassProvider.create().
Before change:
Over 14 samples,
average = 976
median = 969.9
After change:
Over 14 samples,
average = 811.5
median = 813.9
--
MOS_MIGRATED_REVID=89036261
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AutoValueRuleDefinitionMetadata.java b/src/main/java/com/google/devtools/build/lib/analysis/AutoValueRuleDefinitionMetadata.java
new file mode 100644
index 0000000..5356889
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AutoValueRuleDefinitionMetadata.java
@@ -0,0 +1,180 @@
+// Copyright 2014 Google Inc. 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.
+
+/**
+ * TODO(bazel-team): This file has been generated by @AutoValue, and copied here because we do not
+ * support @AutoValue yet. Remove this file and add @AutoValue annotations to RuleDefinition
+ * once possible.
+ */
+package com.google.devtools.build.lib.analysis;
+
+import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
+
+import java.util.List;
+
+import javax.annotation.Generated;
+
+@Generated("com.google.auto.value.processor.AutoValueProcessor")
+final class AutoValueRuleDefinitionMetadata extends RuleDefinition.Metadata {
+
+ private final String name;
+ private final RuleClassType type;
+ private final Class<? extends RuleConfiguredTargetFactory> factoryClass;
+ private final List<Class<? extends RuleDefinition>> ancestors;
+
+ private AutoValueRuleDefinitionMetadata(
+ String name,
+ RuleClassType type,
+ Class<? extends RuleConfiguredTargetFactory> factoryClass,
+ List<Class<? extends RuleDefinition>> ancestors) {
+ if (name == null) {
+ throw new NullPointerException("Null name");
+ }
+ this.name = name;
+ if (type == null) {
+ throw new NullPointerException("Null type");
+ }
+ this.type = type;
+ if (factoryClass == null) {
+ throw new NullPointerException("Null factoryClass");
+ }
+ this.factoryClass = factoryClass;
+ if (ancestors == null) {
+ throw new NullPointerException("Null ancestors");
+ }
+ this.ancestors = ancestors;
+ }
+
+ @Override
+ public String name() {
+ return name;
+ }
+
+ @Override
+ public RuleClassType type() {
+ return type;
+ }
+
+ @Override
+ public Class<? extends RuleConfiguredTargetFactory> factoryClass() {
+ return factoryClass;
+ }
+
+ @Override
+ public List<Class<? extends RuleDefinition>> ancestors() {
+ return ancestors;
+ }
+
+ @Override
+ public String toString() {
+ return "Metadata{"
+ + "name=" + name + ", "
+ + "type=" + type + ", "
+ + "factoryClass=" + factoryClass + ", "
+ + "ancestors=" + ancestors
+ + "}";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o == this) {
+ return true;
+ }
+ if (o instanceof RuleDefinition.Metadata) {
+ RuleDefinition.Metadata that = (RuleDefinition.Metadata) o;
+ return (this.name.equals(that.name()))
+ && (this.type.equals(that.type()))
+ && (this.factoryClass.equals(that.factoryClass()))
+ && (this.ancestors.equals(that.ancestors()));
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ int h = 1;
+ h *= 1000003;
+ h ^= name.hashCode();
+ h *= 1000003;
+ h ^= type.hashCode();
+ h *= 1000003;
+ h ^= factoryClass.hashCode();
+ h *= 1000003;
+ h ^= ancestors.hashCode();
+ return h;
+ }
+
+ static final class Builder extends RuleDefinition.Metadata.Builder {
+ private String name;
+ private RuleClassType type;
+ private Class<? extends RuleConfiguredTargetFactory> factoryClass;
+ private List<Class<? extends RuleDefinition>> ancestors;
+ Builder() {
+ }
+ Builder(RuleDefinition.Metadata source) {
+ name(source.name());
+ type(source.type());
+ factoryClass(source.factoryClass());
+ ancestors(source.ancestors());
+ }
+ @Override
+ public RuleDefinition.Metadata.Builder name(String name) {
+ this.name = name;
+ return this;
+ }
+ @Override
+ public RuleDefinition.Metadata.Builder type(RuleClassType type) {
+ this.type = type;
+ return this;
+ }
+ @Override
+ public RuleDefinition.Metadata.Builder factoryClass(
+ Class<? extends RuleConfiguredTargetFactory> factoryClass) {
+ this.factoryClass = factoryClass;
+ return this;
+ }
+ @Override
+ public RuleDefinition.Metadata.Builder ancestors(
+ List<Class<? extends RuleDefinition>> ancestors) {
+ this.ancestors = ancestors;
+ return this;
+ }
+ @Override
+ public RuleDefinition.Metadata build() {
+ String missing = "";
+ if (name == null) {
+ missing += " name";
+ }
+ if (type == null) {
+ missing += " type";
+ }
+ if (factoryClass == null) {
+ missing += " factoryClass";
+ }
+ if (ancestors == null) {
+ missing += " ancestors";
+ }
+ if (!missing.isEmpty()) {
+ throw new IllegalStateException("Missing required properties:" + missing);
+ }
+ RuleDefinition.Metadata result = new AutoValueRuleDefinitionMetadata(
+ this.name,
+ this.type,
+ this.factoryClass,
+ this.ancestors);
+ return result;
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index a14cd53..4b7015f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -123,8 +123,6 @@
/**
* A base rule for all test rules.
*/
- @BlazeRule(name = "$test_base_rule",
- type = RuleClassType.ABSTRACT)
public static final class TestBaseRule implements RuleDefinition {
@Override
public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
@@ -169,6 +167,14 @@
.add(attr(":run_under", LABEL).cfg(DATA).value(RUN_UNDER))
.build();
}
+
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name("$test_base_rule")
+ .type(RuleClassType.ABSTRACT)
+ .build();
+ }
}
/**
@@ -205,8 +211,6 @@
/**
* Common parts of rules.
*/
- @BlazeRule(name = "$base_rule",
- type = RuleClassType.ABSTRACT)
public static final class BaseRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
@@ -234,14 +238,19 @@
.add(attr(":action_listener", LABEL_LIST).cfg(HOST).value(ACTION_LISTENER))
.build();
}
+
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name("$base_rule")
+ .type(RuleClassType.ABSTRACT)
+ .build();
+ }
}
/**
* Common ancestor class for all rules.
*/
- @BlazeRule(name = "$rule",
- type = RuleClassType.ABSTRACT,
- ancestors = { BaseRule.class })
public static final class RuleBase implements RuleDefinition {
@Override
public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
@@ -250,6 +259,14 @@
.add(attr("data", LABEL_LIST).cfg(DATA).allowedFileTypes(FileTypeSet.ANY_FILE))
.build();
}
- }
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name("$rule")
+ .type(RuleClassType.ABSTRACT)
+ .ancestors(BaseRule.class)
+ .build();
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BlazeRule.java b/src/main/java/com/google/devtools/build/lib/analysis/BlazeRule.java
deleted file mode 100644
index c349e65..0000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/BlazeRule.java
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright 2014 Google Inc. 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.devtools.build.lib.analysis;
-
-import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
-import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
-
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
-
-/**
- * An annotation for rule classes.
- */
-@Target(ElementType.TYPE)
-@Retention(RetentionPolicy.RUNTIME)
-public @interface BlazeRule {
- /**
- * The name of the rule, as it appears in the BUILD file. If it starts with
- * '$', the rule will be hidden from users and will only be usable from
- * inside Blaze.
- */
- String name();
-
- /**
- * The type of the rule. It can be an abstract rule, a normal rule or a test
- * rule. If the rule type is abstract, the configured class must not be set.
- */
- RuleClassType type() default RuleClassType.NORMAL;
-
- /**
- * The {@link RuleConfiguredTargetFactory} class that implements this rule. If the rule is
- * abstract, this must not be set.
- */
- Class<? extends RuleConfiguredTargetFactory> factoryClass()
- default RuleConfiguredTargetFactory.class;
-
- /**
- * The list of other rule classes this rule inherits from.
- */
- Class<? extends RuleDefinition>[] ancestors() default {};
-}
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 26e0617..ad3cdcd 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
@@ -13,10 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType.ABSTRACT;
import static com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType.TEST;
-import com.google.common.base.Preconditions;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -46,7 +47,6 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -86,6 +86,8 @@
private final Map<String, Class<? extends RuleDefinition>> ruleDefinitionMap =
new HashMap<>();
private final Map<Class<? extends RuleDefinition>, RuleClass> ruleMap = new HashMap<>();
+ private final Map<Class<? extends RuleDefinition>, RuleDefinition> ruleDefinitionInstanceCache =
+ new HashMap<>();
private final Digraph<Class<? extends RuleDefinition>> dependencyGraph =
new Digraph<>();
private ConfigurationCollectionFactory configurationCollectionFactory;
@@ -106,11 +108,12 @@
return this;
}
- public Builder addRuleDefinition(Class<? extends RuleDefinition> ruleDefinition) {
- dependencyGraph.createNode(ruleDefinition);
- BlazeRule annotation = ruleDefinition.getAnnotation(BlazeRule.class);
- for (Class<? extends RuleDefinition> ancestor : annotation.ancestors()) {
- dependencyGraph.addEdge(ancestor, ruleDefinition);
+ public Builder addRuleDefinition(RuleDefinition ruleDefinition) {
+ Class<? extends RuleDefinition> ruleDefinitionClass = ruleDefinition.getClass();
+ ruleDefinitionInstanceCache.put(ruleDefinitionClass, ruleDefinition);
+ dependencyGraph.createNode(ruleDefinitionClass);
+ for (Class<? extends RuleDefinition> ancestor : ruleDefinition.getMetadata().ancestors()) {
+ dependencyGraph.addEdge(ancestor, ruleDefinitionClass);
}
return this;
@@ -148,40 +151,38 @@
}
private RuleClass commitRuleDefinition(Class<? extends RuleDefinition> definitionClass) {
- BlazeRule annotation = definitionClass.getAnnotation(BlazeRule.class);
- Preconditions.checkArgument(ruleClassMap.get(annotation.name()) == null, annotation.name());
+ RuleDefinition instance = checkNotNull(ruleDefinitionInstanceCache.get(definitionClass),
+ "addRuleDefinition(new %s()) should be called before build()", definitionClass.getName());
- Preconditions.checkArgument(
- annotation.type() == ABSTRACT ^
- annotation.factoryClass() != RuleConfiguredTargetFactory.class);
- Preconditions.checkArgument(
- (annotation.type() != TEST) ||
- Arrays.asList(annotation.ancestors()).contains(
- BaseRuleClasses.TestBaseRule.class));
+ RuleDefinition.Metadata metadata = instance.getMetadata();
+ checkArgument(ruleClassMap.get(metadata.name()) == null, metadata.name());
- RuleDefinition instance;
- try {
- instance = definitionClass.newInstance();
- } catch (IllegalAccessException | InstantiationException e) {
- throw new IllegalStateException(e);
- }
- RuleClass[] ancestorClasses = new RuleClass[annotation.ancestors().length];
- for (int i = 0; i < annotation.ancestors().length; i++) {
- ancestorClasses[i] = ruleMap.get(annotation.ancestors()[i]);
+ List<Class<? extends RuleDefinition>> ancestors = metadata.ancestors();
+
+ checkArgument(
+ metadata.type() == ABSTRACT ^ metadata.factoryClass()
+ != RuleConfiguredTargetFactory.class);
+ checkArgument(
+ (metadata.type() != TEST)
+ || ancestors.contains(BaseRuleClasses.TestBaseRule.class));
+
+ RuleClass[] ancestorClasses = new RuleClass[ancestors.size()];
+ for (int i = 0; i < ancestorClasses.length; i++) {
+ ancestorClasses[i] = ruleMap.get(ancestors.get(i));
if (ancestorClasses[i] == null) {
// Ancestors should have been initialized by now
- throw new IllegalStateException("Ancestor " + annotation.ancestors()[i] + " of "
- + annotation.name() + " is not initialized");
+ throw new IllegalStateException("Ancestor " + ancestors.get(i) + " of "
+ + metadata.name() + " is not initialized");
}
}
RuleConfiguredTargetFactory factory = null;
- if (annotation.type() != ABSTRACT) {
- factory = createFactory(annotation.factoryClass());
+ if (metadata.type() != ABSTRACT) {
+ factory = createFactory(metadata.factoryClass());
}
RuleClass.Builder builder = new RuleClass.Builder(
- annotation.name(), annotation.type(), false, ancestorClasses);
+ metadata.name(), metadata.type(), false, ancestorClasses);
builder.factory(factory);
RuleClass ruleClass = instance.build(builder, this);
ruleMap.put(definitionClass, ruleClass);
@@ -193,7 +194,7 @@
public ConfiguredRuleClassProvider build() {
for (Node<Class<? extends RuleDefinition>> ruleDefinition :
- dependencyGraph.getTopologicalOrder()) {
+ dependencyGraph.getTopologicalOrder()) {
commitRuleDefinition(ruleDefinition.getLabel());
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinition.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinition.java
index c5e32e3..77670d5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleDefinition.java
@@ -15,6 +15,12 @@
package com.google.devtools.build.lib.analysis;
import com.google.devtools.build.lib.packages.RuleClass;
+import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
/**
* This class is a common ancestor for every rule object.
@@ -36,4 +42,64 @@
* @return the {@link RuleClass} representing the rule.
*/
RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment);
+
+ /**
+ * Returns metadata for this rule.
+ */
+ Metadata getMetadata();
+
+ /**
+ * Value class that contains the name, type, ancestors of a rule, as well as a reference to the
+ * configured target factory.
+ */
+ public abstract static class Metadata {
+ /**
+ * The name of the rule, as it appears in the BUILD file. If it starts with
+ * '$', the rule will be hidden from users and will only be usable from
+ * inside Blaze.
+ */
+ public abstract String name();
+
+ /**
+ * The type of the rule. It can be an abstract rule, a normal rule or a test
+ * rule. If the rule type is abstract, the configured class must not be set.
+ */
+ public abstract RuleClassType type();
+
+ /**
+ * The {@link RuleConfiguredTargetFactory} class that implements this rule. If the rule is
+ * abstract, this must not be set.
+ */
+ public abstract Class<? extends RuleConfiguredTargetFactory> factoryClass();
+
+ /**
+ * The list of other rule classes this rule inherits from.
+ */
+ public abstract List<Class<? extends RuleDefinition>> ancestors();
+
+ public static Builder builder() {
+ return new AutoValueRuleDefinitionMetadata.Builder()
+ .type(RuleClassType.NORMAL)
+ .factoryClass(RuleConfiguredTargetFactory.class)
+ .ancestors(Collections.<Class<? extends RuleDefinition>>emptyList());
+ }
+
+ public static Metadata empty() {
+ return builder().build();
+ }
+
+ /**
+ * Builder class for the Metadata class.
+ */
+ public abstract static class Builder {
+ public abstract Builder name(String s);
+ public abstract Builder type(RuleClassType type);
+ public abstract Builder factoryClass(Class<? extends RuleConfiguredTargetFactory> factory);
+ public abstract Builder ancestors(List<Class<? extends RuleDefinition>> ancestors);
+ public Builder ancestors(Class<? extends RuleDefinition>... ancstrs) {
+ return ancestors(Arrays.asList(ancstrs));
+ }
+ public abstract Metadata build();
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java
index dd0ba5b..44bb5e7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigRuleClasses.java
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
-import com.google.devtools.build.lib.analysis.BlazeRule;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.packages.RuleClass;
@@ -40,9 +39,6 @@
/**
* Common settings for all configurability rules.
*/
- @BlazeRule(name = "$config_base_rule",
- type = RuleClass.Builder.RuleClassType.ABSTRACT,
- ancestors = { BaseRuleClasses.BaseRule.class })
public static final class ConfigBaseRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
@@ -55,6 +51,15 @@
"these rules don't include content that gets built into their dependers")
.build();
}
+
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name("$config_base_rule")
+ .type(RuleClass.Builder.RuleClassType.ABSTRACT)
+ .ancestors(BaseRuleClasses.BaseRule.class)
+ .build();
+ }
}
/**
@@ -85,10 +90,6 @@
* themselves inputs to that map. So Bazel has special logic to read and properly apply
* config_setting instances. See {@link ConfiguredTargetFunction#getConfigConditions} for details.
*/
- @BlazeRule(name = "config_setting",
- type = RuleClass.Builder.RuleClassType.NORMAL,
- ancestors = { ConfigBaseRule.class },
- factoryClass = ConfigSetting.class)
public static final class ConfigSettingRule implements RuleDefinition {
/**
* The name of the attribute that declares flag bindings.
@@ -137,6 +138,16 @@
.nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
.build();
}
+
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name("config_setting")
+ .type(RuleClass.Builder.RuleClassType.NORMAL)
+ .ancestors(ConfigBaseRule.class)
+ .factoryClass(ConfigSetting.class)
+ .build();
+ }
}
/*<!-- #BLAZE_RULE (NAME = config_setting, TYPE = OTHER, FAMILY = General)[GENERIC_RULE] -->
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java
index 58116e8..8330dba 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java
@@ -18,7 +18,6 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
-import com.google.devtools.build.lib.analysis.BlazeRule;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.packages.RuleClass;
@@ -28,9 +27,6 @@
/**
* Rule definition for environment rules (for Bazel's constraint enforcement system).
*/
-@BlazeRule(name = EnvironmentRule.RULE_NAME,
- ancestors = { BaseRuleClasses.BaseRule.class },
- factoryClass = Environment.class)
public final class EnvironmentRule implements RuleDefinition {
public static final String RULE_NAME = "environment";
@@ -67,4 +63,13 @@
.setUndocumented()
.build();
}
+
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name(EnvironmentRule.RULE_NAME)
+ .ancestors(BaseRuleClasses.BaseRule.class)
+ .factoryClass(Environment.class)
+ .build();
+ }
}
