Skip to content

Commit

Permalink
Optimize variant transform selection
Browse files Browse the repository at this point in the history 
This should reduce the amount of garbage created during
selection of artifact transforms.
  • Loading branch information
@melix
melix committed May 10, 2019
1 parent 59d2b1f commit a54fceb
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 86 deletions.
4 changes: 3 additions & 1 deletion ...-management/src/main/java/org/gradle/api/internal/artifacts/VariantTransformRegistry.java
View file
Expand Up @@ -22,6 +22,8 @@
import org.gradle.api.artifacts.transform.TransformSpec;
import org.gradle.api.artifacts.transform.VariantTransform;

import java.util.List;

public interface VariantTransformRegistry {

/**
Expand All @@ -33,5 +35,5 @@ public interface VariantTransformRegistry {

<T extends TransformParameters> void registerTransform(Class<? extends TransformAction<T>> actionType, Action<? super TransformSpec<T>> registrationAction);

Iterable<ArtifactTransformRegistration> getTransforms();
List<ArtifactTransformRegistration> getTransforms();
}
3 changes: 1 addition & 2 deletions ...in/java/org/gradle/api/internal/artifacts/transform/AttributeMatchingVariantSelector.java
View file
Expand Up @@ -95,8 +95,7 @@ private ResolvedArtifactSet doSelect(ResolvedVariantSet producer) {
List<Pair<ResolvedVariant, ConsumerVariantMatchResult.ConsumerVariant>> candidates = new ArrayList<Pair<ResolvedVariant, ConsumerVariantMatchResult.ConsumerVariant>>();
for (ResolvedVariant variant : producer.getVariants()) {
AttributeContainerInternal variantAttributes = variant.getAttributes().asImmutable();
ConsumerVariantMatchResult matchResult = new ConsumerVariantMatchResult();
consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, componentRequested, matchResult);
ConsumerVariantMatchResult matchResult = consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, componentRequested);
for (ConsumerVariantMatchResult.ConsumerVariant consumerVariant : matchResult.getMatches()) {
candidates.add(Pair.of(variant, consumerVariant));
}
Expand Down
25 changes: 12 additions & 13 deletions .../main/java/org/gradle/api/internal/artifacts/transform/ConsumerProvidedVariantFinder.java
View file
Expand Up @@ -50,21 +50,20 @@ public ConsumerProvidedVariantFinder(VariantTransformRegistry variantTransforms,
this.attributesFactory = attributesFactory;
}

public void collectConsumerVariants(AttributeContainerInternal actual, AttributeContainerInternal requested, ConsumerVariantMatchResult result) {
public ConsumerVariantMatchResult collectConsumerVariants(AttributeContainerInternal actual, AttributeContainerInternal requested) {
AttributeSpecificCache toCache = getCache(requested);
ConsumerVariantMatchResult cachedResult = toCache.transforms.get(actual);
if (cachedResult == null) {
cachedResult = new ConsumerVariantMatchResult();
findProducersFor(actual, requested, cachedResult);
toCache.transforms.put(actual, cachedResult);
}
cachedResult.applyTo(result);
return toCache.transforms.computeIfAbsent(actual, attrs -> {
return findProducersFor(actual, requested).asImmutable();
});
}

private void findProducersFor(AttributeContainerInternal actual, AttributeContainerInternal requested, ConsumerVariantMatchResult result) {
private ConsumerVariantMatchResult findProducersFor(AttributeContainerInternal actual, AttributeContainerInternal requested) {
// Prefer direct transformation over indirect transformation
List<ArtifactTransformRegistration> candidates = new ArrayList<ArtifactTransformRegistration>();
for (ArtifactTransformRegistration registration : variantTransforms.getTransforms()) {
List<ArtifactTransformRegistration> transforms = variantTransforms.getTransforms();
int nbOfTransforms = transforms.size();
ConsumerVariantMatchResult result = new ConsumerVariantMatchResult(nbOfTransforms * nbOfTransforms);
for (ArtifactTransformRegistration registration : transforms) {
if (matchAttributes(registration.getTo(), requested)) {
if (matchAttributes(actual, registration.getFrom())) {
ImmutableAttributes variantAttributes = attributesFactory.concat(actual.asImmutable(), registration.getTo().asImmutable());
Expand All @@ -76,13 +75,12 @@ private void findProducersFor(AttributeContainerInternal actual, AttributeContai
}
}
if (result.hasMatches()) {
return;
return result;
}

for (ArtifactTransformRegistration candidate : candidates) {
ConsumerVariantMatchResult inputVariants = new ConsumerVariantMatchResult();
AttributeContainerInternal requestedPrevious = computeRequestedAttributes(requested, candidate);
collectConsumerVariants(actual, requestedPrevious, inputVariants);
ConsumerVariantMatchResult inputVariants = collectConsumerVariants(actual, requestedPrevious);
if (!inputVariants.hasMatches()) {
continue;
}
Expand All @@ -92,6 +90,7 @@ private void findProducersFor(AttributeContainerInternal actual, AttributeContai
result.matched(variantAttributes, transformation, inputVariant.depth + 1);
}
}
return result;
}

private AttributeContainerInternal computeRequestedAttributes(AttributeContainerInternal result, ArtifactTransformRegistration transform) {
Expand Down
18 changes: 14 additions & 4 deletions ...src/main/java/org/gradle/api/internal/artifacts/transform/ConsumerVariantMatchResult.java
View file
Expand Up @@ -16,19 +16,25 @@

package org.gradle.api.internal.artifacts.transform;

import com.google.common.collect.Lists;
import org.gradle.api.internal.attributes.AttributeContainerInternal;
import org.gradle.api.internal.attributes.ImmutableAttributes;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;

public class ConsumerVariantMatchResult {
private int minDepth;
private final List<ConsumerVariant> matches = new ArrayList<ConsumerVariant>();
private final List<ConsumerVariant> matches;

public void applyTo(ConsumerVariantMatchResult result) {
result.matches.addAll(this.matches);
ConsumerVariantMatchResult(int estimateSize) {
matches = Lists.newArrayListWithExpectedSize(estimateSize);
}

private ConsumerVariantMatchResult(ConsumerVariantMatchResult other) {
this.minDepth = other.minDepth;
this.matches = Collections.unmodifiableList(other.matches);
}

public void matched(ImmutableAttributes output, Transformation transformation, int depth) {
Expand All @@ -52,6 +58,10 @@ public Collection<ConsumerVariant> getMatches() {
return matches;
}

public ConsumerVariantMatchResult asImmutable() {
return new ConsumerVariantMatchResult(this);
}

public static class ConsumerVariant {
final AttributeContainerInternal attributes;
final Transformation transformation;
Expand Down
2 changes: 1 addition & 1 deletion ...ain/java/org/gradle/api/internal/artifacts/transform/DefaultVariantTransformRegistry.java
View file
Expand Up @@ -121,7 +121,7 @@ private static void validateAttributes(RecordingRegistration registration) {
}
}

public Iterable<ArtifactTransformRegistration> getTransforms() {
public List<ArtifactTransformRegistration> getTransforms() {
return transforms;
}

Expand Down
71 changes: 39 additions & 32 deletions ...y/org/gradle/api/internal/artifacts/transform/AttributeMatchingVariantSelectorSpec.groovy
View file
Expand Up @@ -21,6 +21,7 @@ import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.Resol
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ResolvedVariant
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ResolvedVariantSet
import org.gradle.api.internal.attributes.AttributesSchemaInternal
import org.gradle.api.internal.attributes.ImmutableAttributes
import org.gradle.api.internal.attributes.ImmutableAttributesFactory
import org.gradle.api.internal.tasks.TaskDependencyResolveContext
import org.gradle.internal.Describables
Expand Down Expand Up @@ -137,8 +138,8 @@ class AttributeMatchingVariantSelectorSpec extends Specification {
}
})
1 * attributeMatcher.matches(_, _) >> Collections.emptyList()
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, Mock(Transformation), 1)
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, Mock(Transformation), 1)
}
}

Expand Down Expand Up @@ -185,11 +186,11 @@ class AttributeMatchingVariantSelectorSpec extends Specification {
})
1 * attributeMatcher.matches(_, _) >> Collections.emptyList()
1 * attributeMatcher.isMatching(requestedAttributes, requestedAttributes) >> true
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform1, 2)
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform1, 2)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform2, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform2, 3)
}
1 * attributeMatcher.matches(_, _) >> { args -> args[0] }
}
Expand Down Expand Up @@ -237,11 +238,11 @@ class AttributeMatchingVariantSelectorSpec extends Specification {
})
1 * attributeMatcher.matches(_, _) >> Collections.emptyList()
1 * attributeMatcher.isMatching(requestedAttributes, requestedAttributes) >> true
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform1, 2)
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform1, 2)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform2, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform2, 3)
}
1 * attributeMatcher.matches(_, _) >> { args -> args[0] }
}
Expand Down Expand Up @@ -288,11 +289,11 @@ class AttributeMatchingVariantSelectorSpec extends Specification {
}
})
1 * attributeMatcher.matches(_, _) >> Collections.emptyList()
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(otherVariantAttributes, transform1, 2)
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes) >> { args ->
match(otherVariantAttributes, transform1, 2)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(variantAttributes, transform2, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes) >> { args ->
match(variantAttributes, transform2, 3)
}
1 * attributeMatcher.matches(_, _) >> { args -> [args[0].get(0)] }
}
Expand Down Expand Up @@ -346,14 +347,14 @@ class AttributeMatchingVariantSelectorSpec extends Specification {
})
1 * attributeMatcher.matches(_, _) >> Collections.emptyList()
2 * attributeMatcher.isMatching(requestedAttributes, requestedAttributes) >> true
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform1, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform1, 3)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform2, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform2, 3)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(yetAnotherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform3, 2)
1 * consumerProvidedVariantFinder.collectConsumerVariants(yetAnotherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform3, 2)
}
1 * attributeMatcher.matches(_, _) >> { args -> args[0] }
}
Expand Down Expand Up @@ -406,14 +407,14 @@ class AttributeMatchingVariantSelectorSpec extends Specification {
}
})
1 * attributeMatcher.matches(_, _) >> Collections.emptyList()
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform1, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform1, 3)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform2, 2)
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform2, 2)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(yetAnotherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform3, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(yetAnotherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform3, 3)
}
2 * attributeMatcher.isMatching(requestedAttributes, requestedAttributes) >> true
1 * attributeMatcher.matches(_, _) >> { args -> args[0] }
Expand Down Expand Up @@ -466,16 +467,22 @@ class AttributeMatchingVariantSelectorSpec extends Specification {
}
})
1 * attributeMatcher.matches(_, _) >> Collections.emptyList()
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform1, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(variantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform1, 3)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform2, 2)
1 * consumerProvidedVariantFinder.collectConsumerVariants(otherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform2, 2)
}
1 * consumerProvidedVariantFinder.collectConsumerVariants(yetAnotherVariantAttributes, requestedAttributes, _) >> { args ->
args[2].matched(requestedAttributes, transform3, 3)
1 * consumerProvidedVariantFinder.collectConsumerVariants(yetAnotherVariantAttributes, requestedAttributes) >> { args ->
match(requestedAttributes, transform3, 3)
}
1 * attributeMatcher.matches(_, _) >> { args -> args[0] }
3 * attributeMatcher.isMatching(requestedAttributes, requestedAttributes) >>> [false, false, true]
}

static ConsumerVariantMatchResult match(ImmutableAttributes output, Transformation trn, int depth) {
def result = new ConsumerVariantMatchResult(2)
result.matched(output, trn, depth)
result
}
}

0 comments on commit a54fceb

Please sign in to comment.