Skip to content

Commit

Permalink
Filter use-constraint violating providers of packages
Browse files Browse the repository at this point in the history
If a provider is chosen for a given requirement this can imply that also
other providers are now become invalid, currently the resolver simply
checks the whole state and adds new permutations what is rather
expensive.

This now adds an additional local filtering step that checks for the
chosen provider for the requirement and discards any incompatible
provider for other packages in the same resource.

As a result for a small testcase this saves 3 out of 23 (~ 10%) uses
permutations.
  • Loading branch information
laeubi committed Sep 11, 2024
1 parent 36ccbc1 commit 81e4423
Show file tree
Hide file tree
Showing 12 changed files with 356 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3959,7 +3959,8 @@ protected void assertNotMoreThanPermutationCreated(ResolutionReport report,
fail("Maximum of " + max + " permutations expected but was " + permutations);
} else if (permutations < max) {
System.out.println(
"## Permutations are below the threshold, consider adjusting the testcase to assert the lower count!");
"## Permutations (" + permutations + ") are below the threshold (" + max
+ "), consider adjusting the testcase to assert the lower count!");
}
return;
}
Expand Down Expand Up @@ -4367,8 +4368,8 @@ private static void assertWires(List<ModuleWire> required, List<ModuleWire>... p
public void testLocalUseConstraintViolations() throws Exception {
ResolutionReport result = resolveTestSet("set1");
// TODO get down permutation count!
assertSucessfulWith(result, 52);
assertNotMoreThanPermutationCreated(result, ResolutionReport::getSubstitutionPermutations, 23);
assertSucessfulWith(result, 49);
assertNotMoreThanPermutationCreated(result, ResolutionReport::getSubstitutionPermutations, 20);
}

private ResolutionReport resolveTestSet(String name) throws Exception {
Expand Down
4 changes: 2 additions & 2 deletions bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2
Export-Package: org.eclipse.core.runtime.adaptor;x-friends:="org.eclipse.core.runtime",
org.eclipse.core.runtime.internal.adaptor;x-internal:=true,
org.eclipse.equinox.log;version="1.1";uses:="org.osgi.framework,org.osgi.service.log",
org.eclipse.osgi.container;version="1.7.0";
org.eclipse.osgi.container;version="1.8.0";
uses:="org.eclipse.osgi.report.resolution,
org.osgi.framework.wiring,
org.eclipse.osgi.framework.eventmgr,
Expand Down Expand Up @@ -107,7 +107,7 @@ Bundle-Activator: org.eclipse.osgi.internal.framework.SystemBundleActivator
Bundle-Description: %systemBundle
Bundle-Copyright: %copyright
Bundle-Vendor: %eclipse.org
Bundle-Version: 3.21.0.qualifier
Bundle-Version: 3.21.100.qualifier
Bundle-Localization: systembundle
Bundle-DocUrl: http://www.eclipse.org
Eclipse-ExtensibleAPI: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,34 @@ public static String toString(Capability capability) {
return Constants.PROVIDE_CAPABILITY + ": " + capability.toString(); //$NON-NLS-1$
}

/**
* Generates a human readable string representation of the the given
* {@link Resource} using the IDENTITY_NAMESPACE
*
* @param resource the {@link Resource} for which a string representation is
* desired
*
* @since 3.21
*/
public static String toString(Resource resource) {
String id = null;
Version version = null;
List<Capability> caps = resource.getCapabilities(null);
for (Capability cap : caps) {
if (cap.getNamespace().equals(IdentityNamespace.IDENTITY_NAMESPACE)) {
id = cap.getAttributes().get(IdentityNamespace.IDENTITY_NAMESPACE).toString();
version = (Version) cap.getAttributes().get(IdentityNamespace.CAPABILITY_VERSION_ATTRIBUTE);
}
}
if (id != null) {
if (version != null) {
return String.format("%s %s", id, version); //$NON-NLS-1$
}
return id;
}
return resource.toString();
}

private static String createOSGiCapability(Capability cap) {
Map<String, Object> attributes = new HashMap<>(cap.getAttributes());
Map<String, String> directives = cap.getDirectives();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static org.eclipse.osgi.internal.container.NamespaceList.WIRE;

import java.security.Permission;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -37,6 +38,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import org.apache.felix.resolver.Logger;
import org.apache.felix.resolver.PermutationType;
import org.apache.felix.resolver.ResolutionError;
Expand Down Expand Up @@ -504,6 +506,74 @@ public void logUsesConstraintViolation(Resource resource, ResolutionError error)
}
}

public void logRequirement(String message, Requirement requirement) {
debug(String.format(message, ModuleContainer.toString(requirement)));
}

public void logCapability(String message, Capability requirement) {
debug(String.format(message, ModuleContainer.toString(requirement)));
}

@Override
public void logCandidates(Resource resource, Function<Requirement, List<Capability>> candidateLookup) {
if (DEBUG_USES) {
Wiring wiring = getWirings().get(resource);
List<Requirement> reqs = (wiring != null) ? wiring.getResourceRequirements(null)
: resource.getRequirements(null);
List<Requirement> dreqs = (wiring != null)
? getDynamicRequirements(wiring.getResourceRequirements(null))
: getDynamicRequirements(resource.getRequirements(null));
boolean hasMulti = hasMulti(reqs, candidateLookup);
Debug.println(String.format(" %s%s (%s)", getMultiMarker(hasMulti), //$NON-NLS-1$
ModuleContainer.toString(resource),
((wiring != null) ? "RESOLVED)" : "UNRESOLVED)"))); //$NON-NLS-1$ //$NON-NLS-2$
printRe(reqs, candidateLookup);
printRe(dreqs, candidateLookup);
}
}

private String getMultiMarker(boolean hasMulti) {
return hasMulti ? "[?]" : "[!]"; //$NON-NLS-1$ //$NON-NLS-2$
}

private boolean hasMulti(List<Requirement> reqs, Function<Requirement, List<Capability>> candidateLookup) {
for (Requirement req : reqs) {
List<Capability> remaining = candidateLookup.apply(req);
if (remaining.size() > 1) {
return true;
}
}
return false;
}

private int printRe(List<Requirement> reqs, Function<Requirement, List<Capability>> candidateLookup) {
int dup = 0;
for (Requirement req : reqs) {
List<Capability> remaining = candidateLookup.apply(req);
boolean hasMulti = remaining.size() > 1;
dup++;
Debug.println(MessageFormat.format(" {0}{1}: ", getMultiMarker(hasMulti), //$NON-NLS-1$
ModuleContainer.toString(req)));
for (Capability cap : remaining) {
Debug.println(String.format(" %s", ModuleContainer.toString(cap))); //$NON-NLS-1$
}
}
return dup;
}

private List<Requirement> getDynamicRequirements(List<Requirement> reqs) {
List<Requirement> result = new ArrayList<>();
if (reqs != null) {
for (Requirement req : reqs) {
String resolution = req.getDirectives().get(PackageNamespace.REQUIREMENT_RESOLUTION_DIRECTIVE);
if ((resolution != null) && resolution.equals(PackageNamespace.RESOLUTION_DYNAMIC)) {
result.add(req);
}
}
}
return result;
}

Map<Resource, ResolutionException> getUsesConstraintViolations() {
return errors == null ? Collections.emptyMap() : errors;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.felix.resolver;

import java.io.PrintStream;
import java.util.*;
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicBoolean;
Expand All @@ -34,6 +35,8 @@

class Candidates
{
private static final boolean FILTER_USES = Boolean
.parseBoolean(System.getProperty("felix.resolver.candidates.filteruses", "true"));
static class PopulateResult {
boolean success;
ResolutionError error;
Expand Down Expand Up @@ -709,7 +712,7 @@ public Capability getFirstCandidate(Requirement req)
return null;
}

public void removeFirstCandidate(Requirement req)
public Capability removeFirstCandidate(Requirement req)
{
CandidateSelector candidates = m_candidateMap.get(req);
// Remove the conflicting candidate.
Expand All @@ -721,6 +724,7 @@ public void removeFirstCandidate(Requirement req)
// Update the delta with the removed capability
CopyOnWriteSet<Capability> capPath = m_delta.getOrCompute(req);
capPath.add(cap);
return cap;
}

public CandidateSelector clearMultipleCardinalityCandidates(Requirement req, Collection<Capability> caps)
Expand Down Expand Up @@ -1145,54 +1149,15 @@ public Candidates copy()
m_delta.deepClone());
}

public void dump(ResolveContext rc)
{
// Create set of all revisions from requirements.
Set<Resource> resources = new CopyOnWriteSet<Resource>();
for (Entry<Requirement, CandidateSelector> entry
: m_candidateMap.entrySet())
{
resources.add(entry.getKey().getResource());
}
// Now dump the revisions.
System.out.println("=== BEGIN CANDIDATE MAP ===");
for (Resource resource : resources)
{
Wiring wiring = rc.getWirings().get(resource);
System.out.println(" " + resource
+ " (" + ((wiring != null) ? "RESOLVED)" : "UNRESOLVED)"));
List<Requirement> reqs = (wiring != null)
? wiring.getResourceRequirements(null)
: resource.getRequirements(null);
for (Requirement req : reqs)
{
CandidateSelector candidates = m_candidateMap.get(req);
if ((candidates != null) && (!candidates.isEmpty()))
{
System.out.println(" " + req + ": " + candidates);
}
}
reqs = (wiring != null)
? Util.getDynamicRequirements(wiring.getResourceRequirements(null))
: Util.getDynamicRequirements(resource.getRequirements(null));
for (Requirement req : reqs)
{
CandidateSelector candidates = m_candidateMap.get(req);
if ((candidates != null) && (!candidates.isEmpty()))
{
System.out.println(" " + req + ": " + candidates);
}
}
}
System.out.println("=== END CANDIDATE MAP ===");
}

public Candidates permutate(Requirement req)
{
if (!Util.isMultiple(req) && canRemoveCandidate(req))
{
Candidates perm = copy();
perm.removeFirstCandidate(req);
Capability removed = perm.removeFirstCandidate(req);
if (FILTER_USES) {
ProblemReduction.removeUsesViolations(perm, req, m_session.getLogger());
}
return perm;
}
return null;
Expand Down Expand Up @@ -1341,4 +1306,25 @@ public ResolutionException toException() {

}

/**
* Returns the current provided {@link Capability} for the given resource if it
* is a candidate for the {@link Requirement}
*
* @param resource the resource to check
* @param requirement the requirement to check
* @return the {@link Capability} this Resource currently provides for the given
* {@link Requirement} or <code>null</code> if none is provided.
*/
public Capability getCapability(Resource resource, Requirement requirement) {
List<Capability> providers = getCandidates(requirement);
if (providers != null) {
for (Capability capability : providers) {
if (capability.getResource().equals(resource)) {
return capability;
}
}
}
return null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
*/
package org.apache.felix.resolver;

import java.util.List;
import java.util.function.Function;
import org.osgi.resource.Capability;
import org.osgi.resource.Requirement;
import org.osgi.resource.Resource;

/**
Expand Down Expand Up @@ -132,6 +136,31 @@ public void logUsesConstraintViolation(Resource resource, ResolutionError error)
// do nothing by default
}

/**
* Called to debug the current mapping of {@link Requirement}s to
* {@link Capability}s in a resolve operation
*
* @param resource the resource for this log message
* @param candidateLookup a mapping between a requirement and a list of all
* current candidate {@link Capability}s eligible for
* resolving
*/
public void logCandidates(Resource resource,
Function<Requirement, List<Capability>> candidateLookup)
{
// do nothing by default
}

public void logRequirement(String message, Requirement requirement)
{
debug(String.format(message, requirement));
}

public void logCapability(String message, Capability requirement)
{
debug(String.format(message, requirement));
}

/**
* Called whenever a new permutation is added by the resolver.
*
Expand Down
Loading

0 comments on commit 81e4423

Please sign in to comment.