Bug 14722

Created attachment 2770 [details]
Sample implementation of the lookup

I would like to achieve two things:
1. During standalone use of the library this Lookup would be the
default one instead of EMPTY
2. When used in IDE it should update its state if module set is
changed.

The simplest solution for me would be to have two implementations -
one private in org.openide.util that would be simple - does not need
to update its state, because classloader is still the same

Second in org.netbeans.core that would listen on changes of
classloader and update appropriatelly.

What do you think Jesse? Will we made it for 3.3?

Depends on #16041 because right now the findResources does not work in
modules classloader...

I agree with what you say the requirements for this should be. However
I do not think this should be attempted for 3.3. We are late in the
release cycle and what bug in existing functionality does this fix?

Target milestone -> 3.3.1.

Target milestone -> 3.3.1.

I would appreciate if you could implement this as soon as possible. I 
would like to use it in standalone MDR.

OK, will try to work on this soon.

The biggest problem is probably the implementation for modules. Either
one has to listen on Lookup.lookup (ClassLoader.class) change or the
code has to be duplicated in core or it has to be called by reflection
everytime a module sets get updated...

Listening to a change in ClassLoader is not enough. That does not help
when modules are added. So I would vote for duplication or reflection,
unless you want to add e.g.

public static Lookup Lookup.getServicesLookup(ClassLoader l);

whereby core could proxy to this and change it after any module
enablement change. Lookup.EMPTY would be an alias for this with the
startup classloader.

Do you think it is worth an API addition?

First problem is with Lookup.EMPTY being alias of services lookup -
not good - some modules rely on Lookup.EMPTY to be really empty.

As concern the API addition, I would vote for adding the method as
package private for begining and calling it by reflection with a
comment that it can be made public when we recieve a request for that.

Such request may appear, but I do not see anybody else than core who
would need it right now.

OK. BTW I think I meant to say that "default value of
Lookup.getDefault() will be such a lookup, based on startup
classloader" rather than saying that Lookup.EMPTY would be changed.

I have got it working. But it is, pardon the expression, butt slow.
Everything slows down from this. Thread dumps show that
ClassLoader.getResources() (either our impl or JDK's, don't remember)
is taking all the extra time; the rest of the code should be fairly
efficient. I will create a branch for the patch so it can be worked on.

Note that #19443 could work with just the openide side of the patch,
and leave out the core side of the patch which is additional.

Created attachment 4490 [details]
Working patch

CVS branch manifest_lookup_jan_2002 in core + openide holds the
current work.

Obviously trouble in core.modules.ProxyClassLoader findResources. The
comment: 

   // Don't bother optimizing this call by domains--it is practically
unused.

explains it all. But I am affraid I do not know enough to improve it.
Just suggestions:

keep track of JARs that have META-INF/services and ignore others?

Hey, that's my comment! I knew I should have done it right then. :-)

Yes, some optimizations such as you suggest should work. I just need
to spend a little more time on it.

Fixed up ProxyClassLoader.getResources which was written incorrectly.
That seems to have improved things, but still turning on
META-INF/services/ lookup on modules during startup makes a startup
and automatic close 3 seconds slower. Time for OptimizeIt! I think.

Good fix (I have not known about the instanceof check) now I spend in
metalookup 608ms during statup (used to be 20-30s). 

Who knows where the rest of the time is spend...

With the fix in issue #20036 applied, using manifest lookup from
modules seems to slow down total IDE startup time by about 250msec, or
around 1%. I assume this is mostly time spent looking in ZIP files. It
might be time spent changing lookup, but I tried to batch this
together with the lookup change fired when FolderLookup is ready and
the built-in ErrorManager lookup is removed. I don't know how to
analyze exactly what the extra time is doing; if I could get profiles
from two IDE runs, with and without, and subtract them and only look
at the difference, that would be very helpful - I don't know if that
is possible with OptimizeIt! though.

Measurement technique: use the option
-J-Dnetbeans.lookup.nometainfservices=false to use the service
(default), =true to suppress it. Make a full build and then run some
primer runs and then loop:

for stage in primer 1 2 3 4 5 6 7 8 9 10; do for suppress in true
false; do echo stage=$stage suppress=$suppress; time
./netbeans/bin/runide.sh -userdir testuserdir
-J-Dnetbeans.lookup.nometainfservices=$suppress
-J-Dnetbeans.close=true; done; done

Time per run is pretty variable, generally 19-20 seconds though
sometimes as many as 25; the primer and first two runs were much
slower for some reason; over runs 3-10, the averaged time with
services on was 240msec more; in 1 out of these 8 cases the time with
services on was a little bit less, so obviously measurements are not
very precise. Linux 2.2.12, 256Mb, JDK 1.3.1_02, running Gnome and
Emacs only.

Petr, any opinion? It is possible to merge just the openide part of
the patch, which would make such lookup usable from libraries; it is
also possible to merge the core part but leave it off by default, so
we could see if anyone needs it from modules, and if so try harder to
optimize it.

Silence, so I propose to merge the openide half as is, and merge the
core half as well (incl. tests) but make it disabled by default (using
a system property). If we decide we need such lookup from modules in
the future, we can make it enabled by default and check for
optimizations then.

Wonder if it ever will become enabled... I will try to work on speed
up based on issue 17722, it can also speed up this issue.

Good point. Well, for now no modules *need* services from manifest.
The only things you can actually look up this way are MIDI mixers :-)
and JAXP factories.

As described.

committed   * Up-To-Date  1.135      
core/src/org/netbeans/core/NbTopManager.java
committed   * Up-To-Date  1.5        
core/src/org/netbeans/core/modules/JarClassLoader.java
committed   * Up-To-Date  1.9        
core/src/org/netbeans/core/modules/ProxyClassLoader.java
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/MetaInfServicesTest.java
committed   * Up-To-Date  1.3        
core/test/unit/src/org/netbeans/core/lookup/data/build.xml
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/data/services-jar-1.mf
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/data/services-jar-2.mf
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/data/services-jar-1/META-INF/services/org.foo.Interface
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/data/services-jar-1/org/foo/Interface.java
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/data/services-jar-1/org/foo/impl/Implementation1.java
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/data/services-jar-2/META-INF/services/org.foo.Interface
committed   * Up-To-Date  1.2        
core/test/unit/src/org/netbeans/core/lookup/data/services-jar-2/org/bar/Implementation2.java
committed   * Up-To-Date  1.14       
openide/src/org/openide/util/Lookup.java
committed   * Up-To-Date  1.2        
openide/src/org/openide/util/MetaInfServicesLookup.java
committed   * Up-To-Date  1.36        openide/test/build.xml
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/MetaInfServicesLookupTest.java
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/data/.cvsignore
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/data/build.xml
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/data/services-jar-1/META-INF/services/org.foo.Interface
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/data/services-jar-1/org/foo/Interface.java
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/data/services-jar-1/org/foo/impl/Implementation1.java
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/data/services-jar-2/META-INF/services/org.foo.Interface
committed   * Up-To-Date  1.2        
openide/test/unit/src/org/openide/util/data/services-jar-2/org/bar/Implementation2.java

change lookup/core -> lookup/openide

Metainf integrated, right now in org.openide.util.lookup