Skip to content

Commit

Permalink
Minor performance improvements in dependency management (#30217)
Browse files Browse the repository at this point in the history
  • Loading branch information
jvandort authored Aug 20, 2024
2 parents f33df4d + af97131 commit 678cf70
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ class RecordingVariantSet(
return EmptySchema.INSTANCE
}

override fun getVariants(): Set<ResolvedVariant> {
return setOf(this)
override fun getVariants(): List<ResolvedVariant> {
return listOf(this)
}

override fun getOverriddenAttributes(): ImmutableAttributes {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableList;
import org.gradle.api.Describable;
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.internal.attributes.AttributesSchemaInternal;
Expand All @@ -30,9 +30,9 @@ public class DefaultResolvedVariantSet implements ResolvedVariantSet {
private final ComponentIdentifier componentIdentifier;
private final AttributesSchemaInternal schema;
private final ImmutableAttributes selectionAttributes;
private final ImmutableSet<ResolvedVariant> variants;
private final ImmutableList<ResolvedVariant> variants;

public DefaultResolvedVariantSet(ComponentIdentifier componentIdentifier, AttributesSchemaInternal schema, ImmutableAttributes selectionAttributes, ImmutableSet<ResolvedVariant> variants) {
public DefaultResolvedVariantSet(ComponentIdentifier componentIdentifier, AttributesSchemaInternal schema, ImmutableAttributes selectionAttributes, ImmutableList<ResolvedVariant> variants) {
this.componentIdentifier = componentIdentifier;
this.schema = schema;
this.selectionAttributes = selectionAttributes;
Expand Down Expand Up @@ -65,7 +65,7 @@ public AttributesSchemaInternal getSchema() {
}

@Override
public ImmutableSet<ResolvedVariant> getVariants() {
public ImmutableList<ResolvedVariant> getVariants() {
return variants;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import javax.annotation.Nonnull;
import java.io.File;
import java.util.Collections;
import java.util.List;
import java.util.Set;

/**
Expand Down Expand Up @@ -232,8 +233,8 @@ public DisplayName asDescribable() {
}

@Override
public Set<ResolvedVariant> getVariants() {
return Collections.singleton(this);
public List<ResolvedVariant> getVariants() {
return Collections.singletonList(this);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.gradle.api.internal.attributes.ImmutableAttributes;

import javax.annotation.Nullable;
import java.util.Set;
import java.util.List;

/**
* Represents some provider of {@link ResolvedVariant} instances to select from.
Expand All @@ -42,9 +42,9 @@ public interface ResolvedVariantSet {
AttributesSchemaInternal getSchema();

/**
* The variants available for artifact selection when variant reselection is not enabled.
* The variants available for artifact selection.
*/
Set<ResolvedVariant> getVariants();
List<ResolvedVariant> getVariants();

/**
* The provider may have been selected thanks to a different attribute set than the one from
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableList;
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.artifacts.component.ProjectComponentIdentifier;
import org.gradle.api.capabilities.Capability;
Expand Down Expand Up @@ -60,7 +60,7 @@ public class VariantResolvingArtifactSet implements ArtifactSet {
private final GraphVariantSelector graphVariantSelector;
private final AttributesSchemaInternal consumerSchema;

private final Lazy<ImmutableSet<ResolvedVariant>> ownArtifacts = Lazy.locking().of(this::calculateOwnArtifacts);
private final Lazy<ImmutableList<ResolvedVariant>> ownArtifacts = Lazy.locking().of(this::calculateOwnArtifacts);

public VariantResolvingArtifactSet(
VariantArtifactResolver variantResolver,
Expand Down Expand Up @@ -98,7 +98,7 @@ public ResolvedArtifactSet select(
return ResolvedArtifactSet.EMPTY;
}

ImmutableSet<ResolvedVariant> variants;
ImmutableList<ResolvedVariant> variants;
try {
if (!spec.getSelectFromAllVariants()) {
variants = ownArtifacts.get();
Expand Down Expand Up @@ -126,11 +126,11 @@ private ResolvedArtifactSet asTransformed(ResolvedVariant sourceVariant, Variant
}
}

public ImmutableSet<ResolvedVariant> calculateOwnArtifacts() {
public ImmutableList<ResolvedVariant> calculateOwnArtifacts() {
if (artifacts.isEmpty()) {
return getArtifactsForGraphVariant(variant);
} else {
return ImmutableSet.of(variant.prepareForArtifactResolution().resolveAdhocVariant(variantResolver, artifacts));
return ImmutableList.of(variant.prepareForArtifactResolution().resolveAdhocVariant(variantResolver, artifacts));
}
}

Expand All @@ -142,7 +142,7 @@ public ImmutableSet<ResolvedVariant> calculateOwnArtifacts() {
* same algorithm used during graph variant selection. This considers requested and declared
* capabilities.</p>
*/
private ImmutableSet<ResolvedVariant> getArtifactVariantsForReselection(ImmutableAttributes requestAttributes) {
private ImmutableList<ResolvedVariant> getArtifactVariantsForReselection(ImmutableAttributes requestAttributes) {
// First, find the graph variant containing the artifact variants to select among.
VariantGraphResolveState graphVariant = graphVariantSelector.selectByAttributeMatchingLenient(
requestAttributes,
Expand All @@ -155,7 +155,7 @@ private ImmutableSet<ResolvedVariant> getArtifactVariantsForReselection(Immutabl
// It is fine if no graph variants satisfy our request.
// Variant reselection allows no target variants to be found.
if (graphVariant == null) {
return ImmutableSet.of();
return ImmutableList.of();
}

// Next, return all artifact variants for the selected graph variant.
Expand All @@ -165,10 +165,10 @@ private ImmutableSet<ResolvedVariant> getArtifactVariantsForReselection(Immutabl
/**
* Resolve all artifact variants for the given graph variant.
*/
private ImmutableSet<ResolvedVariant> getArtifactsForGraphVariant(VariantGraphResolveState graphVariant) {
private ImmutableList<ResolvedVariant> getArtifactsForGraphVariant(VariantGraphResolveState graphVariant) {
VariantArtifactResolveState variantState = graphVariant.prepareForArtifactResolution();
Set<? extends VariantResolveMetadata> artifactVariants = variantState.getArtifactVariants();
ImmutableSet.Builder<ResolvedVariant> resolved = ImmutableSet.builderWithExpectedSize(artifactVariants.size());
ImmutableList.Builder<ResolvedVariant> resolved = ImmutableList.builderWithExpectedSize(artifactVariants.size());

ComponentArtifactResolveMetadata componentMetadata = component.prepareForArtifactResolution().getArtifactMetadata();
if (exclusions.mayExcludeArtifacts()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,10 @@

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

import com.google.common.collect.ImmutableList;
import org.gradle.api.attributes.Attribute;
import org.gradle.api.attributes.HasAttributes;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.BrokenResolvedArtifactSet;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ResolvedArtifactSet;
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.AttributeContainerInternal;
import org.gradle.api.internal.attributes.AttributeValue;
import org.gradle.api.internal.attributes.AttributesSchemaInternal;
import org.gradle.api.internal.attributes.ImmutableAttributes;
import org.gradle.api.internal.attributes.ImmutableAttributesFactory;
Expand All @@ -33,9 +28,7 @@
import org.gradle.internal.component.resolution.failure.ResolutionFailureHandler;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

/**
* A {@link ArtifactVariantSelector} that uses attribute matching to select a matching set of artifacts.
Expand Down Expand Up @@ -82,15 +75,12 @@ public ResolvedArtifactSet select(ResolvedVariantSet producer, ImmutableAttribut
private ResolvedArtifactSet doSelect(ResolvedVariantSet producer, boolean allowNoMatchingVariants, ResolvedArtifactTransformer resolvedArtifactTransformer, AttributeMatchingExplanationBuilder explanationBuilder, ImmutableAttributes requestAttributes) {
AttributeMatcher matcher = schema.withProducer(producer.getSchema());
ImmutableAttributes componentRequested = attributesFactory.concat(requestAttributes, producer.getOverriddenAttributes());
final List<ResolvedVariant> variants = ImmutableList.copyOf(producer.getVariants());
final List<ResolvedVariant> variants = producer.getVariants();

List<? extends ResolvedVariant> matches = matcher.matchMultipleCandidates(variants, componentRequested, explanationBuilder);
if (matches.size() == 1) {
return matches.get(0).getArtifacts();
} else if (matches.size() > 1) {
// Request is ambiguous. Rerun matching again, except capture an explanation this time for reporting.
TraceDiscardedVariants newExpBuilder = new TraceDiscardedVariants();
matches = matcher.matchMultipleCandidates(variants, componentRequested, newExpBuilder);
throw failureProcessor.ambiguousArtifactsFailure(schema, matcher, producer, componentRequested, matches);
}

Expand Down Expand Up @@ -159,28 +149,4 @@ private static List<TransformedVariant> tryDisambiguate(

return differentTransforms;
}

private static class TraceDiscardedVariants implements AttributeMatchingExplanationBuilder {

private final Set<HasAttributes> discarded = new HashSet<>();

@Override
public boolean canSkipExplanation() {
return false;
}

@Override
public <T extends HasAttributes> void candidateDoesNotMatchAttributes(T candidate, AttributeContainerInternal requested) {
recordDiscardedCandidate(candidate);
}

public <T extends HasAttributes> void recordDiscardedCandidate(T candidate) {
discarded.add(candidate);
}

@Override
public <T extends HasAttributes> void candidateAttributeDoesNotMatch(T candidate, Attribute<?> attribute, Object requestedValue, AttributeValue<?> candidateValue) {
recordDiscardedCandidate(candidate);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,19 @@ private boolean allCommonAttributesSatisfy(
return true;
}

return requested.keySet().stream().map(schema::tryRehydrate).allMatch(attribute -> {
for (Attribute<?> attribute : requested.keySet()) {
AttributeValue<?> requestedAttributeValue = requested.findEntry(attribute);
AttributeValue<?> candidateAttributeValue = candidate.findEntry(attribute.getName());
AttributeValue<?> requestedAttributeValue = requested.findEntry(attribute.getName());

return !candidateAttributeValue.isPresent() || !requestedAttributeValue.isPresent() ||
predicate.test(attribute, requestedAttributeValue, candidateAttributeValue);
});
if (candidateAttributeValue.isPresent()) {
Attribute<?> typedAttribute = schema.tryRehydrate(attribute);
if (!predicate.test(typedAttribute, requestedAttributeValue, candidateAttributeValue)) {
return false;
}
}
}

return true;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class AttributeMatchingArtifactVariantSelectorSpec extends Specification {

1 * variantSet.getSchema() >> attributesSchema
1 * variantSet.getOverriddenAttributes() >> ImmutableAttributes.EMPTY
2 * attributeMatcher.matchMultipleCandidates(_, _, _) >> [variant, otherVariant]
1 * attributeMatcher.matchMultipleCandidates(_, _, _) >> [variant, otherVariant]
2 * attributeMatcher.isMatchingValue(_, _, _) >> true
0 * consumerProvidedVariantFinder._
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class DefaultArtifactVariantSelectorFactoryTest extends Specification {
def variant2 = resolvedVariant()
def variant1Artifacts = Stub(ResolvedArtifactSet)
def set = resolvedVariantSet()
def variants = [variant1, variant2] as Set
def variants = [variant1, variant2]

given:
set.schema >> producerSchema
Expand All @@ -78,7 +78,7 @@ class DefaultArtifactVariantSelectorFactoryTest extends Specification {
def variant1 = resolvedVariant()
def variant2 = resolvedVariant()
def set = resolvedVariantSet()
def variants = [variant1, variant2] as Set
def variants = [variant1, variant2]

given:
set.asDescribable() >> Describables.of('<component>')
Expand Down Expand Up @@ -109,7 +109,7 @@ class DefaultArtifactVariantSelectorFactoryTest extends Specification {
def variant1 = resolvedVariant()
def variant2 = resolvedVariant()
def set = resolvedVariantSet()
def variants = [variant1, variant2] as Set
def variants = [variant1, variant2]
def transformedVariants = variants.collect { transformedVariant(it, requested)}

given:
Expand Down Expand Up @@ -151,7 +151,7 @@ Found the following transforms:
def variant1 = resolvedVariant()
def variant2 = resolvedVariant()
def set = resolvedVariantSet()
def variants = [variant1, variant2] as Set
def variants = [variant1, variant2]

given:
set.schema >> producerSchema
Expand All @@ -173,7 +173,7 @@ Found the following transforms:
def variant1 = resolvedVariant()
def variant2 = resolvedVariant()
def set = resolvedVariantSet()
def variants = [variant1, variant2] as Set
def variants = [variant1, variant2]

given:
set.schema >> producerSchema
Expand Down

0 comments on commit 678cf70

Please sign in to comment.