felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Felix Meschberger" <Felix.Meschber...@day.com>
Subject Re: Framework changes
Date Wed, 24 Jan 2007 19:42:41 GMT
On 1/24/07, Richard S. Hall <heavy@ungoverned.org> wrote:
>
> I instead did the following, since pkgName is used later in the method:


Oops, forgot about that. This solution, though, looks ok.

Regards
Felix

        // Ignore any dynamic requirements whose packages don't match.
>         String dynPkgName = ((Requirement) dynamics[i]).getPackageName();
>         boolean wildcard = (dynPkgName.lastIndexOf(".*") >= 0);
>         dynPkgName = (wildcard)
>             ? dynPkgName.substring(0, dynPkgName.length() - 2) :
> dynPkgName;
>         if (dynPkgName.equals("*") ||
>             pkgName.equals(dynPkgName) ||
>             (wildcard && pkgName.startsWith(dynPkgName + ".")))
>         {
>             // Find candidate...
>         }
>
> -> richard
>
> Felix Meschberger wrote:
> > Hi Richard,
> >
> > Just from looking at the code, it seems ok. Also keeping the trailing
> dot
> > for the prefix check seems reasonable. But leaving the dot would fail
> the
> > equality check.
> >
> > Given dynPkgName="com.package.*" and pkgName="com.package". In this
> case,
> > the dynPkgName would be shortened to "com.package.", which would neither
> > evaluate to true for pkgName.equals(dynPkgName) nor
> > pkgName.startsWith(dynPkgName).
> > [ Given that "com.package.*" should match "com.package". ]
> >
> > How about this solution :
> >
> >               // Ignore any dynamic requirements whose packages don't
> > match.
> >               String dynPkgName = ((Requirement)
> > dynamics[i]).getPackageName();
> >               boolean wildcard = (dynPkgName.lastIndexOf(".*") >= 0);
> >               if (wildcard)
> >               {
> >                    dynPkgName = dynPkgName.substring(0,
> > dynPkgName.length()
> > - 1):
> >                    pkgName += ".";
> >               {
> >               if (dynPkgName.equals("*") ||
> >                   pkgName.equals(dynPkgName) ||
> >                   (wildcard && pkgName.startsWith(dynPkgName)))
> >               {
> >                   // Find candidate...
> >               }
> >
> > Regards
> > Felix
> >
> >
> > On 1/24/07, Richard S. Hall <heavy@ungoverned.org> wrote:
> >>
> >> Richard S. Hall wrote:
> >> > Felix,
> >> >
> >> > Your patch seems ok for now, although I would like to eventually
> >> > optimize this away if possible.
> >> >
> >> > I have modified your patch slightly to make it a positive test,
> rather
> >> > than a negative test, and have also extended it to catch more cases.
> >> > Here is the gist of the patch now, let me know if it looks like it
> >> > will work for you:
> >> >
> >> >                // Ignore any dynamic requirements whose packages
> don't
> >> > match.
> >> >                String dynPkgName = ((Requirement)
> >> > dynamics[i]).getPackageName();
> >> >                boolean wildcard = (dynPkgName.lastIndexOf(".*") >=
> 0);
> >> >                dynPkgName = (wildcard)
> >> >                    ? dynPkgName.substring(0, dynPkgName.length() - 2)
> >> > : dynPkgName;
> >> >                if (dynPkgName.equals("*") ||
> >> >                    pkgName.equals(dynPkgName) ||
> >> >                    (wildcard && pkgName.startsWith(dynPkgName)))
> >> >                {
> >> >                    // Find candidate...
> >> >                }
> >>
> >> Actually, I think that should be "- 1" instead of "- 2", because we
> will
> >> want to keep the "." so when we do "startsWith()" it will correctly
> >> match package names.
> >>
> >> -> richard
> >> >
> >> > -> richard
> >> >
> >> > Felix Meschberger wrote:
> >> >> HI Richard,
> >> >>
> >> >> I updated my local framework copy from SVN today and noticed, that
> >> >> performance degraded remarkably for bundles using
> >> DynamicImport-Package.
> >> >> After looking around a while, I saw, that
> >> >> R4SearchPolicyCore.attemptDynamicImport looks after dynamic
> >> >> Requirements and
> >> >> builds a filter from the requirement filter and the requested
> >> >> package. This
> >> >> filter is then used to scan all bundles ... Using this over and over
> >> >> is very
> >> >> expensive.
> >> >>
> >> >> I wonder, whether this small (somewhat hacky) fix is legal [ at
> least
> >> it
> >> >> does seem to work for me, yet I am not sure, whether this is
> >> acceptable)
> >> >>
> >> >> --------------------------------
> >> >> Index:
> >> >>
> >>
> src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java
> >>
> >> >>
> >> >> ===================================================================
> >> >> ---
> >> >>
> >>
> src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java
> >>
> >> >>
> >> >> (revision 499005)
> >> >> +++
> >> >>
> >>
> src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java
> >>
> >> >>
> >> >> (working copy)
> >> >> @@ -510,6 +510,17 @@
> >> >>                 // is necessary because we cannot easily determine
> >> which
> >> >>                 // package name a given dynamic requirement matches,
> >> >> since
> >> >>                 // it is only a filter.
> >> >> +
> >> >> +                // do not consider dynamic packags not matching the
> >> >> requested
> >> >> +                if ( ICapability.PACKAGE_NAMESPACE.equals(
> >> >> dynamics[i].getNamespace() ) )
> >> >> +                {
> >> >> +                    String dynPack = ( ( Requirement ) dynamics[i]
> >> >> ).getPackageName();
> >> >> +                    if ( !pkgName.equals( dynPack ) && !(
pkgName +
> >> >> ".*"
> >> >> ).equals( dynPack ) )
> >> >> +                    {
> >> >> +                        continue;
> >> >> +                    }
> >> >> +                }
> >> >> +
> >> >>                 IRequirement req = null;
> >> >>                 try
> >> >>                 {
> >> >> -------------------------------
> >> >>
> >> >>
> >> >> What do you think ?
> >> >>
> >> >> Regards and Thanks
> >> >> Felix
> >> >>
> >> >> PS: Yes I know, that this is work in progress :-)
> >> >>
> >> >> On 1/22/07, Richard S. Hall <heavy@ungoverned.org> wrote:
> >> >>>
> >> >>> Steven E. Harris wrote:
> >> >>> > "Richard S. Hall" <heavy@ungoverned.org> writes:
> >> >>> >
> >> >>> >
> >> >>> >> In short, the module loader abstraction previously worked
in
> >> terms
> >> >>> >> of exports/imports, but now it has been generalized to
work in
> >> terms
> >> >>> >> of capabilities/requirements. A capability is simply a
set of
> >> >>> >> properties, while a requirement is a filter over a set
of
> >> >>> >> properties.
> >> >>> >>
> >> >>> >
> >> >>> > Does this mean that some of the capabilities and requirements
> >> types
> >> >>> > from the OBR bundle will be moved (or copied) into Felix proper?
> >> >>> >
> >> >>>
> >> >>> Essentially, yes. I have had to make some modifications to their
> >> >>> implementation for the time being, but I hope to continue to work
> on
> >> >>> them to make them more generic like the original OBR types.
> >> >>>
> >> >>> -> richard
> >> >>>
> >> >>
> >>
> >
>

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