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 May 11, 2024
1 parent 5fa3aed commit 63b2053
Show file tree
Hide file tree
Showing 4 changed files with 339 additions and 30 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
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,7 +1149,15 @@ public Candidates copy()
m_delta.deepClone());
}

public void dump(ResolveContext rc)
/**
* Dump the current candidate set to system out
*
* @param rc the resolve context that should be used to look for existing
* wirings
* @param all if true all requirements are printed, if false only those that
* have more than one provider
*/
public void dump(ResolveContext rc, boolean all, PrintStream printStream)
{
// Create set of all revisions from requirements.
Set<Resource> resources = new CopyOnWriteSet<Resource>();
Expand All @@ -1155,44 +1167,70 @@ public void dump(ResolveContext rc)
resources.add(entry.getKey().getResource());
}
// Now dump the revisions.
System.out.println("=== BEGIN CANDIDATE MAP ===");
printStream.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)
dumpResource(resource, rc, all, printStream);
}
printStream.println("=== END CANDIDATE MAP ===");
}

protected void dumpResource(Resource resource, ResolveContext rc, boolean all, PrintStream printStream) {
Wiring wiring = rc == null ? null : rc.getWirings().get(resource);
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);
List<Requirement> dreqs = (wiring != null) ? Util.getDynamicRequirements(wiring.getResourceRequirements(null))
: Util.getDynamicRequirements(resource.getRequirements(null));
boolean hasMulti = hasMulti(reqs);
printStream.println(" " + (hasMulti ? "[?]" : "[!]") + Util.getResourceName(resource) + " ("
+ ((wiring != null) ? "RESOLVED)" : "UNRESOLVED)"));
if (all || hasMulti) {
printRe(reqs, printStream, all);
printRe(dreqs, printStream, all);
}
}

private boolean hasMulti(List<Requirement> reqs) {
for (Requirement req : reqs) {
CandidateSelector candidates = m_candidateMap.get(req);
if ((candidates != null) && (!candidates.isEmpty())) {
List<Capability> remaining = candidates.getRemainingCandidates();
if (remaining.size() > 1) {
return true;
}
}
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);
}
return false;
}

protected int printRe(List<Requirement> reqs, PrintStream printStream, boolean all) {
int dup = 0;
for (Requirement req : reqs) {
CandidateSelector candidates = m_candidateMap.get(req);
if ((candidates != null) && (!candidates.isEmpty())) {
List<Capability> remaining = candidates.getRemainingCandidates();
boolean hasMulti = remaining.size() > 1;
if (all || hasMulti) {
dup++;
printStream.println(" " + (hasMulti ? "[?]" : "[!]") + Util.toString(req) + ": ");
for (Capability cap : remaining) {
printStream.println(" " + Util.toString(cap));
}
}
}
}
System.out.println("=== END CANDIDATE MAP ===");
return dup;
}

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);
}
return perm;
}
return null;
Expand Down Expand Up @@ -1341,4 +1379,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
@@ -0,0 +1,121 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.felix.resolver;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import org.eclipse.osgi.container.ModuleContainer;
import org.osgi.framework.namespace.PackageNamespace;
import org.osgi.resource.Capability;
import org.osgi.resource.Requirement;
import org.osgi.resource.Resource;

/**
* The idea of the {@link ProblemReduction} class is to strike out
* {@link Capability}s that might satisfy {@link Requirement}s but violates some
* contracts that would lead to a guaranteed unresolvable state.
*/
class ProblemReduction {

private static final Capability[] EMPTY_CAPABILITIES = new Capability[0];

private static final boolean DEBUG_VIOLATES = false;

/**
* Removes all violating providers for a given {@link Requirement} and
* {@link Candidates} in a local search, that is if the requirement has any uses
* it checks if there are other packages used by this one and removes any
* offending providers from the top of the list.
*
* @param candidates candidates to filter
* @param requirement the requirement where the search should start
* @return a list of Candidates that where dropped as part of the filtering
*/
static List<Candidates> removeUsesViolations(Candidates candidates, Requirement requirement) {
Resource targetResource = requirement.getResource();
// fetch the current candidate for this requirement
Capability currentCandidate = candidates.getFirstCandidate(requirement);
Resource candidateResource = currentCandidate.getResource();
// now check if it has any uses constraints
Set<String> uses = new TreeSet<>(Util.getUses(currentCandidate));
if (uses.isEmpty()) {
// there is nothing this one can conflict in this current set of candidates
return Collections.emptyList();
}
if (DEBUG_VIOLATES) {
System.out.println("=== remove uses violations for " + ModuleContainer.toString(requirement));
System.out.println("== current candidate is " + ModuleContainer.toString(currentCandidate));
candidates.dumpResource(targetResource, null, true, System.out);
}
boolean repeat;
int round = 0;
List<Candidates> dropped = new ArrayList<>();
do {
repeat = false;
round++;
if (DEBUG_VIOLATES) {
System.out.println("Round " + round + ":");
for (String usedPackage : uses) {
System.out.println(" uses: " + usedPackage);
}
}
// now look at all other imports of the target resource if it is a package that
// is part of a used package
for (Requirement packageRequirement : targetResource.getRequirements(PackageNamespace.PACKAGE_NAMESPACE)) {
if (packageRequirement == requirement) {
continue;
}
Capability providedPackage = candidates.getCapability(candidateResource, packageRequirement);
if (providedPackage == null) {
// we do not provide anything for this package
continue;
}
if (uses.contains(Util.getPackageName(providedPackage))) {
// this is a package where we are a candidate and that has a uses constraint, so
// this package must be provided by us as well or we run into a uses-violation
// later on!
Capability capability = removeViolators(candidates, candidateResource, packageRequirement, dropped);
// if we have added any additional uses we need to reiterate...
repeat |= uses.addAll(Util.getUses(capability));
}
}
} while (repeat);
if (DEBUG_VIOLATES && !dropped.isEmpty()) {
System.out.println();
System.out.println("After removal (" + dropped.size() + " dropped)");
candidates.dumpResource(targetResource, null, true, System.out);
System.out.println();
}
return dropped;
}

private static Capability removeViolators(Candidates candidates, Resource candidateResource,
Requirement packageRequirement, List<Candidates> dropped) {
Capability capability;
while ((capability = candidates.getFirstCandidate(packageRequirement)).getResource() != candidateResource) {
dropped.add(candidates.copy());
candidates.removeFirstCandidate(packageRequirement);
}
return capability;
}

}
Loading

0 comments on commit 63b2053

Please sign in to comment.