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/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());
}
