felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guillaume Nodet <gno...@apache.org>
Subject Re: svn commit: r1628627 - in /felix/trunk/resolver/src/main/java/org/apache/felix/resolver: Candidates.java ResolverImpl.java
Date Wed, 01 Oct 2014 07:44:31 GMT
I've reverted this commit as I did not intend to commit it to trunk.
It's pending review and testing on a github branch:
    https://github.com/gnodet/felix/commits/resolver-improvements

2014-10-01 9:39 GMT+02:00 <gnodet@apache.org>:

> Author: gnodet
> Date: Wed Oct  1 07:39:44 2014
> New Revision: 1628627
>
> URL: http://svn.apache.org/r1628627
> Log:
> Isolate Candidates internal structure from resolver
>
> Modified:
>
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java
>
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
>
> Modified:
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java
> URL:
> http://svn.apache.org/viewvc/felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java?rev=1628627&r1=1628626&r2=1628627&view=diff
>
> ==============================================================================
> ---
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java
> (original)
> +++
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java
> Wed Oct  1 07:39:44 2014
> @@ -444,7 +444,7 @@ class Candidates
>              Requirement substitutedReq =
> m_subtitutableMap.get(substituteStatus.getKey());
>              if (substitutedReq != null)
>              {
> -                ResolverImpl.permutateIfNeeded(this, substitutedReq,
> importPermutations);
> +                permutateIfNeeded(substitutedReq, importPermutations);
>              }
>              Set<Requirement> dependents =
> m_dependentMap.get(substituteStatus.getKey());
>              if (dependents != null)
> @@ -480,7 +480,7 @@ class Candidates
>                          {
>                              if (Util.isOptional(dependent))
>                              {
> -                                clearCandidates(dependent);
> +                                m_candidateMap.remove(dependent);
>                              }
>                              else
>                              {
> @@ -781,12 +781,35 @@ class Candidates
>       */
>      public List<Capability> getCandidates(Requirement req)
>      {
> -        return m_candidateMap.get(req);
> +        return Collections.unmodifiableList(m_candidateMap.get(req));
>      }
>
> -    public void clearCandidates(Requirement req)
> +    public Capability getFirstCandidate(Requirement req)
>      {
> -        m_candidateMap.remove(req);
> +        List<Capability> candidates = m_candidateMap.get(req);
> +        if (candidates != null && !candidates.isEmpty())
> +        {
> +            return m_candidateMap.get(req).get(0);
> +        }
> +        return null;
> +    }
> +
> +    public void removeFirstCandidate(Requirement req)
> +    {
> +        List<Capability> candidates = m_candidateMap.get(req);
> +        // Remove the conflicting candidate.
> +        candidates.remove(0);
> +        if (candidates.isEmpty())
> +        {
> +            m_candidateMap.remove(req);
> +        }
> +    }
> +
> +    public List<Capability> clearCandidates(Requirement req,
> Collection<Capability> caps)
> +    {
> +        List<Capability> l = m_candidateMap.get(req);
> +        l.removeAll(caps);
> +        return l;
>      }
>
>      /**
> @@ -1260,4 +1283,50 @@ class Candidates
>          }
>          System.out.println("=== END CANDIDATE MAP ===");
>      }
> +
> +    public void permutate(Requirement req, List<Candidates> permutations)
> +    {
> +        if (!Util.isMultiple(req) && canRemoveCandidate(req))
> +        {
> +            Candidates perm = copy();
> +            perm.removeFirstCandidate(req);
> +            permutations.add(perm);
> +        }
> +    }
> +
> +    public boolean canRemoveCandidate(Requirement req)
> +    {
> +        List<Capability> candidates = m_candidateMap.get(req);
> +        return ((candidates != null) && (candidates.size() > 1 ||
> Util.isOptional(req)));
> +    }
> +
> +    public void permutateIfNeeded(Requirement req, List<Candidates>
> permutations)
> +    {
> +        List<Capability> candidates = m_candidateMap.get(req);
> +        if ((candidates != null) && (candidates.size() > 1))
> +        {
> +            // Check existing permutations to make sure we haven't
> +            // already permutated this requirement. This check for
> +            // duplicate permutations is simplistic. It assumes if
> +            // there is any permutation that contains a different
> +            // initial candidate for the requirement in question,
> +            // then it has already been permutated.
> +            boolean permutated = false;
> +            for (Candidates existingPerm : permutations)
> +            {
> +                List<Capability> existingPermCands =
> existingPerm.m_candidateMap.get(req);
> +                if (existingPermCands != null &&
> !existingPermCands.get(0).equals(candidates.get(0)))
> +                {
> +                    permutated = true;
> +                }
> +            }
> +            // If we haven't already permutated the existing
> +            // import, do so now.
> +            if (!permutated)
> +            {
> +                permutate(req, permutations);
> +            }
> +        }
> +    }
> +
>  }
>
> Modified:
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
> URL:
> http://svn.apache.org/viewvc/felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java?rev=1628627&r1=1628626&r2=1628627&view=diff
>
> ==============================================================================
> ---
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
> (original)
> +++
> felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
> Wed Oct  1 07:39:44 2014
> @@ -244,8 +244,7 @@ public class ResolverImpl implements Res
>                          List<Requirement> hostReq =
> hostReqs.get(resource);
>                          if (hostReq != null)
>                          {
> -                            target =
> allCandidates.getCandidates(hostReq.get(0))
> -                                .iterator().next().getResource();
> +                            target =
> allCandidates.getFirstCandidate(hostReq.get(0)).getResource();
>                          }
>
>                          calculatePackageSpaces(
> @@ -351,8 +350,7 @@ public class ResolverImpl implements Res
>                          List<Requirement> hostReq =
> hostReqs.get(resource);
>                          if (hostReq != null)
>                          {
> -                            target =
> allCandidates.getCandidates(hostReq.get(0))
> -                                .iterator().next().getResource();
> +                            target =
> allCandidates.getFirstCandidate(hostReq.get(0)).getResource();
>                          }
>
>                          if (allCandidates.isPopulated(target))
> @@ -908,17 +906,19 @@ public class ResolverImpl implements Res
>                              req.getDirectives()
>
>  .get(BundleNamespace.REQUIREMENT_VISIBILITY_DIRECTIVE);
>                          if ((value != null)
> -                            &&
> value.equals(BundleNamespace.VISIBILITY_REEXPORT)
> -                            && (allCandidates.getCandidates(req) != null))
> +                            &&
> value.equals(BundleNamespace.VISIBILITY_REEXPORT))
>                          {
> -                            mergeCandidatePackages(
> -                                rc,
> -                                current,
> -                                currentReq,
> -
> allCandidates.getCandidates(req).iterator().next(),
> -                                resourcePkgMap,
> -                                allCandidates,
> -                                cycles);
> +                            Capability cap =
> allCandidates.getFirstCandidate(req);
> +                            if (cap != null) {
> +                                mergeCandidatePackages(
> +                                        rc,
> +                                        current,
> +                                        currentReq,
> +                                        cap,
> +                                        resourcePkgMap,
> +                                        allCandidates,
> +                                        cycles);
> +                            }
>                          }
>                      }
>                  }
> @@ -1150,9 +1150,9 @@ public class ResolverImpl implements Res
>                      else if
> (!sourceBlame.m_cap.getResource().equals(blame.m_cap.getResource()))
>                      {
>                          // Try to permutate the conflicting requirement.
> -                        permutate(allCandidates, blame.m_reqs.get(0),
> importPermutations);
> +                        allCandidates.permutate(blame.m_reqs.get(0),
> importPermutations);
>                          // Try to permutate the source requirement.
> -                        permutate(allCandidates,
> sourceBlame.m_reqs.get(0), importPermutations);
> +
> allCandidates.permutate(sourceBlame.m_reqs.get(0), importPermutations);
>                          // Report conflict.
>                          ResolutionException ex = new ResolutionException(
>                              "Uses constraint violation. Unable to resolve
> resource "
> @@ -1248,17 +1248,9 @@ public class ResolverImpl implements Res
>                              // See if we can permutate the candidates for
> blamed
>                              // requirement; there may be no candidates if
> the resource
>                              // associated with the requirement is already
> resolved.
> -                            List<Capability> candidates =
> permutation.getCandidates(req);
> -                            if ((candidates != null) &&
> (candidates.size() > 1 || Util.isOptional(req)))
> -                            {
> +                            if (permutation.canRemoveCandidate(req)) {
> +                                permutation.removeFirstCandidate(req);
>                                  mutated.add(req);
> -                                // Remove the conflicting candidate.
> -                                candidates.remove(0);
> -                                if (candidates.isEmpty())
> -                                {
> -                                    permutation.clearCandidates(req);
> -                                }
> -                                // Continue with the next uses constraint.
>                                  break;
>                              }
>                          }
> @@ -1361,17 +1353,9 @@ public class ResolverImpl implements Res
>                              // See if we can permutate the candidates for
> blamed
>                              // requirement; there may be no candidates if
> the resource
>                              // associated with the requirement is already
> resolved.
> -                            List<Capability> candidates =
> permutation.getCandidates(req);
> -                            if ((candidates != null) &&
> (candidates.size() > 1 || Util.isOptional(req)))
> -                            {
> +                            if (permutation.canRemoveCandidate(req)) {
> +                                permutation.removeFirstCandidate(req);
>                                  mutated.add(req);
> -                                // Remove the conflicting candidate.
> -                                candidates.remove(0);
> -                                if (candidates.isEmpty())
> -                                {
> -                                    permutation.clearCandidates(req);
> -                                }
> -                                // Continue with the next uses constraint.
>                                  break;
>                              }
>                          }
> @@ -1404,7 +1388,7 @@ public class ResolverImpl implements Res
>                              // with existing import decisions, we may end
> up trying
>                              // to permutate the same import a lot of
> times, so we should
>                              // try to check if that the case and only
> permutate it once.
> -                            permutateIfNeeded(allCandidates, req,
> importPermutations);
> +                            allCandidates.permutateIfNeeded(req,
> importPermutations);
>                          }
>                      }
>
> @@ -1427,10 +1411,9 @@ public class ResolverImpl implements Res
>          int permCount = usesPermutations.size() +
> importPermutations.size();
>          for (Requirement req : resource.getRequirements(null))
>          {
> -            List<Capability> cands = allCandidates.getCandidates(req);
> -            if (cands != null && !cands.isEmpty())
> +            Capability cap = allCandidates.getFirstCandidate(req);
> +            if (cap != null)
>              {
> -                Capability cap = cands.get(0);
>                  if (!resource.equals(cap.getResource()))
>                  {
>                      try
> @@ -1447,7 +1430,7 @@ public class ResolverImpl implements Res
>                          // to backtrack on our current candidate
> selection.
>                          if (permCount == (usesPermutations.size() +
> importPermutations.size()))
>                          {
> -                            permutate(allCandidates, req,
> importPermutations);
> +                            allCandidates.permutate(req,
> importPermutations);
>                          }
>                          throw ex;
>                      }
> @@ -1476,64 +1459,13 @@ public class ResolverImpl implements Res
>              }
>              // Get the current candidate list and remove all the
> offending root
>              // cause candidates from a copy of the current permutation.
> -            candidates =
> session.getMultipleCardCandidates().getCandidates(req);
> -            candidates.removeAll(usedBlames.getRootCauses(req));
> +            candidates =
> session.getMultipleCardCandidates().clearCandidates(req,
> usedBlames.getRootCauses(req));
>          }
>          // We only are successful if there is at least one candidate left
>          // for the requirement
>          return (candidates != null) && !candidates.isEmpty();
>      }
>
> -    private static void permutate(
> -        Candidates allCandidates, Requirement req, List<Candidates>
> permutations)
> -    {
> -        if (!Util.isMultiple(req))
> -        {
> -            List<Capability> candidates =
> allCandidates.getCandidates(req);
> -            if ((candidates != null) && (candidates.size() > 1 ||
> Util.isOptional(req)))
> -            {
> -                Candidates perm = allCandidates.copy();
> -                candidates = perm.getCandidates(req);
> -                candidates.remove(0);
> -                if (candidates.isEmpty())
> -                {
> -                    perm.clearCandidates(req);
> -                }
> -                permutations.add(perm);
> -            }
> -        }
> -    }
> -
> -    static void permutateIfNeeded(
> -        Candidates allCandidates, Requirement req, List<Candidates>
> permutations)
> -    {
> -        List<Capability> candidates = allCandidates.getCandidates(req);
> -        if ((candidates != null) && (candidates.size() > 1))
> -        {
> -            // Check existing permutations to make sure we haven't
> -            // already permutated this requirement. This check for
> -            // duplicate permutations is simplistic. It assumes if
> -            // there is any permutation that contains a different
> -            // initial candidate for the requirement in question,
> -            // then it has already been permutated.
> -            boolean permutated = false;
> -            for (Candidates existingPerm : permutations)
> -            {
> -                List<Capability> existingPermCands =
> existingPerm.getCandidates(req);
> -                if (existingPermCands != null &&
> !existingPermCands.get(0).equals(candidates.get(0)))
> -                {
> -                    permutated = true;
> -                }
> -            }
> -            // If we haven't already permutated the existing
> -            // import, do so now.
> -            if (!permutated)
> -            {
> -                permutate(allCandidates, req, permutations);
> -            }
> -        }
> -    }
> -
>      private static void calculateExportedPackages(
>          ResolveContext rc,
>          Resource resource,
> @@ -1579,11 +1511,10 @@ public class ResolverImpl implements Res
>                  {
>                      if
> (req.getNamespace().equals(PackageNamespace.PACKAGE_NAMESPACE))
>                      {
> -                        List<Capability> cands =
> allCandidates.getCandidates(req);
> -                        if ((cands != null) && !cands.isEmpty())
> +                        Capability cand =
> allCandidates.getFirstCandidate(req);
> +                        if (cand != null)
>                          {
> -                            String pkgName = (String) cands.get(0)
> -
> .getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE);
> +                            String pkgName = (String)
> cand.getAttributes().get(PackageNamespace.PACKAGE_NAMESPACE);
>                              exports.remove(pkgName);
>                          }
>                      }
> @@ -1804,9 +1735,9 @@ public class ResolverImpl implements Res
>                                  if
> (IdentityNamespace.IDENTITY_NAMESPACE.equals(cand.getNamespace())
>                                      && Util.isFragment(targetCand))
>                                  {
> -                                    targetCand =
> allCandidates.getCandidates(
> -
> targetCand.getRequirements(HostNamespace.HOST_NAMESPACE).get(0))
> -                                        .iterator().next().getResource();
> +                                    targetCand =
> allCandidates.getFirstCandidate(
> +
> targetCand.getRequirements(HostNamespace.HOST_NAMESPACE).get(0))
> +                                        .getResource();
>                                      targetCand =
> allCandidates.getWrappedHost(targetCand);
>                                  }
>
> @@ -1908,12 +1839,10 @@ public class ResolverImpl implements Res
>
>      private static Wire createWire(Requirement requirement, Candidates
> allCandidates)
>      {
> -        List<Capability> candidates =
> allCandidates.getCandidates(requirement);
> -        if (candidates == null || candidates.isEmpty())
> -        {
> +        Capability cand = allCandidates.getFirstCandidate(requirement);
> +        if (cand == null) {
>              return null;
>          }
> -        Capability cand = candidates.get(0);
>          return new WireImpl(
>              getDeclaredResource(requirement.getResource()),
>              getDeclaredRequirement(requirement),
> @@ -1946,9 +1875,8 @@ public class ResolverImpl implements Res
>          List<Wire> packageWires = new ArrayList<Wire>();
>
>          // Get the candidates for the current dynamic requirement.
> -        List<Capability> candCaps = allCandidates.getCandidates(dynReq);
>          // Record the dynamic candidate.
> -        Capability dynCand = candCaps.get(0);
> +        Capability dynCand = allCandidates.getFirstCandidate(dynReq);
>
>          if (!rc.getWirings().containsKey(dynCand.getResource()))
>          {
> @@ -2117,18 +2045,12 @@ public class ResolverImpl implements Res
>      private static Capability getSatisfyingCapability(
>          ResolveContext rc, Candidates allCandidates, Requirement req)
>      {
> -        Capability cap = null;
> -
>          // If the requiring revision is not resolved, then check in the
>          // candidate map for its matching candidate.
> -        List<Capability> cands = allCandidates.getCandidates(req);
> -        if (cands != null)
> -        {
> -            cap = cands.get(0);
> -        }
> +        Capability cap = allCandidates.getFirstCandidate(req);
>          // Otherwise, if the requiring revision is resolved then check
>          // in its wires for the capability satisfying the requirement.
> -        else if (rc.getWirings().containsKey(req.getResource()))
> +        if (cap == null && rc.getWirings().containsKey(req.getResource()))
>          {
>              List<Wire> wires =
>
>  rc.getWirings().get(req.getResource()).getRequiredResourceWires(null);
>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message