felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Richard S. Hall" <he...@ungoverned.org>
Subject Re: Framework changes
Date Wed, 24 Jan 2007 19:52:16 GMT
Felix Meschberger wrote:
> 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.

Great. So, hopefully it fixes your performance issue too! :-)

-> richard

>
> 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
View raw message