Skip to content

Commit

Permalink
Precompute non-configurable attributes at RuleClass level
Browse files Browse the repository at this point in the history
Previously we created this collection for each AggregatingAttributeMapper,
which we create at least every attribute encountered. Calculate the collection
up front to avoid wasting time and memory.

--
MOS_MIGRATED_REVID=109805907
  • Loading branch information
michajlo authored and davidzchen committed Dec 9, 2015
1 parent 6fa20f9 commit cdd5b0e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,13 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
* unconditionally available to computed defaults no matter what dependencies
* they've declared.
*/
private final List<String> nonconfigurableAttributes;
private final List<String> nonConfigurableAttributes;

private AggregatingAttributeMapper(Rule rule) {
super(rule.getPackage(), rule.getRuleClassObject(), rule.getLabel(),
rule.getAttributeContainer());

ImmutableList.Builder<String> nonconfigurableAttributesBuilder = ImmutableList.builder();
for (Attribute attr : rule.getAttributes()) {
if (!attr.isConfigurable()) {
nonconfigurableAttributesBuilder.add(attr.getName());
}
}
nonconfigurableAttributes = nonconfigurableAttributesBuilder.build();
nonConfigurableAttributes = rule.getRuleClassObject().getNonConfigurableAttributes();
}

public static AggregatingAttributeMapper of(Rule rule) {
Expand Down Expand Up @@ -374,7 +368,7 @@ private AttributeMap mapBackedAttributeMap(final Map<String, Object> directMap)
@Override
public <T> T get(String attributeName, Type<T> type) {
owner.checkType(attributeName, type);
if (nonconfigurableAttributes.contains(attributeName)) {
if (nonConfigurableAttributes.contains(attributeName)) {
return owner.get(attributeName, type);
}
if (!directMap.containsKey(attributeName)) {
Expand All @@ -393,7 +387,7 @@ public <T> boolean isConfigurable(String attributeName, Type<T> type) {
@Override public Label getLabel() { return owner.getLabel(); }
@Override public Iterable<String> getAttributeNames() {
return ImmutableList.<String>builder()
.addAll(directMap.keySet()).addAll(nonconfigurableAttributes).build();
.addAll(directMap.keySet()).addAll(nonConfigurableAttributes).build();
}
@Override
public void visitLabels(AcceptsLabelAttribute observer) { owner.visitLabels(observer); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,9 @@ public Object create(Object ruleContext) throws InterruptedException {
*/
private final ImmutableList<Attribute> attributes;

/** Names of the non-configurable attributes of this rule class. */
private final ImmutableList<String> nonConfigurableAttributes;

/**
* The set of implicit outputs generated by a rule, expressed as a function
* of that rule.
Expand Down Expand Up @@ -1083,12 +1086,17 @@ public Object create(Object ruleContext) throws InterruptedException {
this.configurationFragmentPolicy = configurationFragmentPolicy;
this.supportsConstraintChecking = supportsConstraintChecking;

// create the index:
// Create the index and collect non-configurable attributes.
int index = 0;
attributeIndex = new HashMap<>(attributes.length);
ImmutableList.Builder<String> nonConfigurableAttributesBuilder = ImmutableList.builder();
for (Attribute attribute : attributes) {
attributeIndex.put(attribute.getName(), index++);
if (!attribute.isConfigurable()) {
nonConfigurableAttributesBuilder.add(attribute.getName());
}
}
this.nonConfigurableAttributes = nonConfigurableAttributesBuilder.build();
}

private void validateNoClashInPublicNames(Attribute[] attributes) {
Expand Down Expand Up @@ -1214,6 +1222,11 @@ public List<Attribute> getAttributes() {
return attributes;
}

/** Returns set of non-configurable attribute names defined for this class of rule. */
public List<String> getNonConfigurableAttributes() {
return nonConfigurableAttributes;
}

public PredicateWithMessage<Rule> getValidityPredicate() {
return validityPredicate;
}
Expand Down

0 comments on commit cdd5b0e

Please sign in to comment.