Skip to content

Commit

Permalink
Issue #719 Performance regression with fragment additons to classpath
Browse files Browse the repository at this point in the history 
Currently a very simple approach is used that has a very bad
runtime-complexity. This si now improved in the following way:

- whether or not an IU is a fragment is cached
- while iterating, matched fragments are removed
- if the list of fragments is empty (all matched) break out the loop

Signed-off-by: Christoph Läubrich <[email protected]>
  • Loading branch information
@laeubi
laeubi committed Mar 7, 2022
1 parent 7abbc7e commit 3c46a38
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 45 deletions.
18 changes: 9 additions & 9 deletions ...est/src/test/java/org/eclipse/tycho/p2/resolver/P2ResolverAdditionalRequirementsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class P2ResolverAdditionalRequirementsTest {

@Before
public void initBlankResolver() {
impl = new P2ResolverImpl(null, logVerifier.getLogger());
impl = new P2ResolverImpl(null, null, logVerifier.getLogger());
}

@Test
Expand Down Expand Up @@ -92,10 +92,10 @@ public void testZeroVersionInTargetDefinitionUnit() throws Exception {

IInstallableUnit iu = createIU(arbitraryVersion);

Assert.assertTrue("Requires version 0.0.0; should be satisfied by any version", additionalRequirements.get(0)
.isMatch(iu));
Assert.assertTrue("Requires version 0.0.0; should be satisfied by any version", additionalRequirements.get(1)
.isMatch(iu));
Assert.assertTrue("Requires version 0.0.0; should be satisfied by any version",
additionalRequirements.get(0).isMatch(iu));
Assert.assertTrue("Requires version 0.0.0; should be satisfied by any version",
additionalRequirements.get(1).isMatch(iu));
}

@Test
Expand All @@ -111,10 +111,10 @@ public void testNullVersionInTargetDefinitionUnit() throws Exception {

IInstallableUnit iu = createIU(arbitraryVersion);

Assert.assertTrue("Given version was null; should be satisfied by any version", additionalRequirements.get(0)
.isMatch(iu));
Assert.assertTrue("Given version was null; should be satisfied by any version", additionalRequirements.get(1)
.isMatch(iu));
Assert.assertTrue("Given version was null; should be satisfied by any version",
additionalRequirements.get(0).isMatch(iu));
Assert.assertTrue("Given version was null; should be satisfied by any version",
additionalRequirements.get(1).isMatch(iu));
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...cho.p2.resolver.impl.test/src/test/java/org/eclipse/tycho/p2/resolver/P2ResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public class P2ResolverTest extends P2ResolverTestBase {
public void initDefaultResolver() throws Exception {
// org.eclipse.equinox.internal.p2.core.helpers.Tracing.DEBUG_PLANNER_PROJECTOR = true;
pomDependencies = resolverFactory.newPomDependencyCollector();
impl = new P2ResolverImpl(tpFactory, logVerifier.getLogger());
impl = new P2ResolverImpl(tpFactory, null, logVerifier.getLogger());
impl.setEnvironments(getEnvironments());
}

Expand Down
2 changes: 1 addition & 1 deletion ....p2.resolver.impl.test/src/test/java/org/eclipse/tycho/p2/target/TestResolverFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public TargetPlatformFactoryImpl getTargetPlatformFactoryImpl() {

@Override
public P2Resolver createResolver(MavenLogger logger) {
return new P2ResolverImpl(getTargetPlatformFactoryImpl(), mavenContext.getLogger());
return new P2ResolverImpl(getTargetPlatformFactoryImpl(), null, mavenContext.getLogger());
}

@Override
Expand Down
77 changes: 76 additions & 1 deletion ...o.p2.resolver.impl/src/main/java/org/eclipse/tycho/p2/resolver/P2ResolverFactoryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,28 @@
package org.eclipse.tycho.p2.resolver;

import java.io.File;
import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;

import org.eclipse.equinox.internal.p2.metadata.IRequiredCapability;
import org.eclipse.equinox.p2.core.ProvisionException;
import org.eclipse.equinox.p2.metadata.IInstallableUnit;
import org.eclipse.equinox.p2.metadata.IProvidedCapability;
import org.eclipse.equinox.p2.metadata.IRequirement;
import org.eclipse.equinox.p2.publisher.eclipse.BundlesAction;
import org.eclipse.tycho.ArtifactDescriptor;
import org.eclipse.tycho.MavenDependencyDescriptor;
import org.eclipse.tycho.ReactorProject;
Expand All @@ -35,9 +52,11 @@
import org.eclipse.tycho.p2.target.TargetDefinitionResolverService;
import org.eclipse.tycho.p2.target.TargetPlatformFactoryImpl;
import org.eclipse.tycho.p2.target.facade.PomDependencyCollector;
import org.eclipse.tycho.p2.util.resolution.ResolutionData;
import org.eclipse.tycho.repository.local.LocalArtifactRepository;
import org.eclipse.tycho.repository.local.LocalMetadataRepository;

@SuppressWarnings("restriction")
public class P2ResolverFactoryImpl implements P2ResolverFactory {

// TODO cache these instances in an p2 agent, and not here
Expand All @@ -48,6 +67,7 @@ public class P2ResolverFactoryImpl implements P2ResolverFactory {
private LocalRepositoryP2Indices localRepoIndices;
private RemoteAgentManager remoteAgentManager;
private TargetDefinitionResolverService targetDefinitionResolverService;
private ConcurrentMap<IInstallableUnit, Optional<Entry<IInstallableUnit, IRequiredCapability>>> hostRequirementMap = new ConcurrentHashMap<>();

private static synchronized LocalMetadataRepository getLocalMetadataRepository(MavenContext context,
LocalRepositoryP2Indices localRepoIndices) {
Expand Down Expand Up @@ -90,7 +110,62 @@ public TargetPlatformFactoryImpl getTargetPlatformFactory() {

@Override
public P2ResolverImpl createResolver(MavenLogger logger) {
return new P2ResolverImpl(getTargetPlatformFactory(), logger);
return new P2ResolverImpl(getTargetPlatformFactory(), this, logger);
}

public Set<IInstallableUnit> calculateDependencyFragments(ResolutionData data,
Collection<IInstallableUnit> resolvedUnits) {
Collection<IInstallableUnit> availableIUs = data.getAvailableIUs();
List<Entry<IInstallableUnit, IRequiredCapability>> fragmentsList = availableIUs.stream()//
.map(iu -> hostRequirementMap.computeIfAbsent(iu, key -> {
for (IProvidedCapability capability : iu.getProvidedCapabilities()) {
String nameSpace = capability.getNamespace();
if (BundlesAction.CAPABILITY_NS_OSGI_FRAGMENT.equals(nameSpace)) {
String fragmentName = capability.getName();
return findFragmentHostRequirement(iu, fragmentName);
}
}
return Optional.empty();

}))//
.filter(Optional::isPresent)//
.map(Optional::get)//
.collect(Collectors.toCollection(ArrayList::new));
if (fragmentsList.isEmpty()) {
return Collections.emptySet();
}
Set<IInstallableUnit> dependencyFragments = new HashSet<>();
for (Iterator<IInstallableUnit> iterator = resolvedUnits.iterator(); iterator.hasNext()
&& !fragmentsList.isEmpty();) {
IInstallableUnit resolvedUnit = iterator.next();
addMatchingFragments(fragmentsList, dependencyFragments, resolvedUnit);
}
return dependencyFragments;
}

private static void addMatchingFragments(List<Entry<IInstallableUnit, IRequiredCapability>> fragmentsList,
Set<IInstallableUnit> dependencyFragments, IInstallableUnit unitToMatch) {
Iterator<Entry<IInstallableUnit, IRequiredCapability>> iterator = fragmentsList.iterator();
while (iterator.hasNext()) {
Entry<IInstallableUnit, IRequiredCapability> fragment = iterator.next();
if (fragment.getValue().isMatch(unitToMatch)) {
dependencyFragments.add(fragment.getKey());
iterator.remove();
}
}
}

private static Optional<Entry<IInstallableUnit, IRequiredCapability>> findFragmentHostRequirement(
IInstallableUnit unit, String fragmentName) {
for (IRequirement requirement : unit.getRequirements()) {
if (requirement instanceof IRequiredCapability) {
IRequiredCapability requiredCapability = (IRequiredCapability) requirement;
if (fragmentName.equals(requiredCapability.getName())) {
return Optional.of(new SimpleEntry<>(unit, requiredCapability));
}
}
}
return Optional.empty();
}

// setters for DS
Expand Down
40 changes: 7 additions & 33 deletions ...se.tycho.p2.resolver.impl/src/main/java/org/eclipse/tycho/p2/resolver/P2ResolverImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import java.util.function.Consumer;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.equinox.internal.p2.metadata.IRequiredCapability;
import org.eclipse.equinox.internal.p2.metadata.InstallableUnit;
import org.eclipse.equinox.p2.metadata.IArtifactKey;
import org.eclipse.equinox.p2.metadata.IInstallableUnit;
Expand Down Expand Up @@ -98,8 +97,12 @@ public class P2ResolverImpl implements P2Resolver {

private PomDependencies pomDependencies = PomDependencies.ignore;

public P2ResolverImpl(TargetPlatformFactoryImpl targetPlatformFactory, MavenLogger logger) {
private P2ResolverFactoryImpl p2ResolverFactoryImpl;

public P2ResolverImpl(TargetPlatformFactoryImpl targetPlatformFactory, P2ResolverFactoryImpl p2ResolverFactoryImpl,
MavenLogger logger) {
this.targetPlatformFactory = targetPlatformFactory;
this.p2ResolverFactoryImpl = p2ResolverFactoryImpl;
this.logger = logger;
this.monitor = new LoggingProgressMonitor(logger);
this.environments = Collections.singletonList(TargetEnvironment.getRunningEnvironment());
Expand Down Expand Up @@ -255,40 +258,11 @@ protected P2ResolutionResult resolveDependencies(Collection<IInstallableUnit> ro
if (usedTargetPlatformUnits != null) {
usedTargetPlatformUnits.addAll(newState);
}
Set<IInstallableUnit> dependencyFragments = new HashSet<>();
for (IInstallableUnit iu : availableUnits) {
for (IProvidedCapability capability : iu.getProvidedCapabilities()) {
String nameSpace = capability.getNamespace();
if (BundlesAction.CAPABILITY_NS_OSGI_FRAGMENT.equals(nameSpace)) {
String fragmentName = capability.getName();
IRequiredCapability fragmentHost = findFragmentHostRequirement(iu, fragmentName);
if (fragmentHost != null) {
for (IInstallableUnit resolved : newState) {
if (fragmentHost.isMatch(resolved)) {
dependencyFragments.add(iu);
break;
}
}
}
}
}

}
Set<IInstallableUnit> dependencyFragments = p2ResolverFactoryImpl == null ? Collections.emptySet()
: p2ResolverFactoryImpl.calculateDependencyFragments(data, newState);
return toResolutionResult(newState, dependencyFragments, project, targetPlatform);
}

private static IRequiredCapability findFragmentHostRequirement(IInstallableUnit unit, String fragmentName) {
for (IRequirement requirement : unit.getRequirements()) {
if (requirement instanceof IRequiredCapability) {
IRequiredCapability requiredCapability = (IRequiredCapability) requirement;
if (fragmentName.equals(requiredCapability.getName())) {
return requiredCapability;
}
}
}
return null;
}

private P2ResolutionResult toResolutionResult(Collection<IInstallableUnit> resolvedUnits,
Collection<IInstallableUnit> dependencyFragments, ReactorProject project, P2TargetPlatform targetPlatform) {
Set<IInstallableUnit> currentProjectUnits = getProjectUnits(project);
Expand Down

0 comments on commit 3c46a38

Please sign in to comment.