This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 169040 - Module list scanning too slow
Summary: Module list scanning too slow
Status: RESOLVED FIXED
Alias: None
Product: apisupport
Classification: Unclassified
Component: Project (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: rmichalsky
URL:
Keywords: PERFORMANCE
: 169581 170867 170921 170924 171093 172490 172700 (view as bug list)
Depends on: 168450
Blocks:
  Show dependency tree
 
Reported: 2009-07-23 13:02 UTC by Vitezslav Stejskal
Modified: 2009-09-26 21:12 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Full thread dump (32.16 KB, text/plain)
2009-07-23 13:03 UTC, Vitezslav Stejskal
Details
IDE's log file (25.97 KB, text/plain)
2009-07-23 13:04 UTC, Vitezslav Stejskal
Details
Start of a patch (9.79 KB, patch)
2009-08-13 19:16 UTC, Jesse Glick
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vitezslav Stejskal 2009-07-23 13:02:29 UTC
When I invoke goto type dialog (Ctrl+O) and type in a class name (eg. ExtSyntaxSupport, AbstractButton,
AlwaysEnabledAction) it takes really long time to find the class. The threaddumps show activity in
ProjectManager.createProject, apisupport.project.Evaluator.createEvaluator and filesystems. I have around 20 netbeans
modules opened in the IDE and this seems to happen every time I use goto type dialog. The delay is around 20-30 seconds,
which makes the dialog almost useless. I'm going to attach the threaddumps and the log file.
Comment 1 Vitezslav Stejskal 2009-07-23 13:03:21 UTC
Created attachment 85125 [details]
Full thread dump
Comment 2 Vitezslav Stejskal 2009-07-23 13:04:31 UTC
Created attachment 85126 [details]
IDE's log file
Comment 3 Vitezslav Stejskal 2009-07-23 13:07:51 UTC
Product Version: NetBeans IDE Dev (Build 090723)
Java: 1.5.0_14; Java HotSpot(TM) Client VM 1.5.0_14-b03
System: Linux version 2.6.22-16-generic running on i386; UTF-8; en_US (nb)
Userdir: /devel/netbeans/jet-main/nbbuild/testuserdir

I admit that my build is a little bit old and so if this has already been fixed please let me know and I'll use newer
build with my apologies. Thanks
Comment 4 David Strupl 2009-07-23 13:09:26 UTC
Marian reported similar stack trace under 166962 - putting Marian on Cc here.
Comment 5 Vitezslav Stejskal 2009-07-23 13:22:38 UTC
I can't be 100% sure, but this seems to happen when:

1. shutdown the IDE
2. hg fetch
3. ant clean build-nozip
4. start the IDE

After that the goto type is slow, but when the IDE is restarted again the goto type is back to normal. After #4 the IDE
performed the initial scan (eg. up-to-date check and reparse/rescan of modified/new files) and this had already finished
when I used the goto type dialog, so there was no interference with scanning.

Also, the same problem seem to happen when I modify user.build.properties to add/remove logging (ie. I offen add/remove
-J-Dorg....level=FINE to tryme.args in user.build.properties).

Marian's usecase is probably the same, but he hit it from a different place (RepositoryUpdater and scanning).
Comment 6 rmichalsky 2009-07-23 14:36:38 UTC
One problem with opening JARs should be solved by issue #168450, this should provide the main speedup.

This happens when nbbuild module cache is not up-to-date, that is between changing e.g. user.build.properties or some
NB.org project metadata and re-building NetBeans (not sure if clean build is needed), so the workaround is to do a
(clean?) build of NB.

I will investigate if there are any recent regressions and in general why whole NB.org sources are scanned instead of
just transitive module deps, but I guess this has been in apisupport since beginning so I doubt this can be changed
easily. Maybe Jesse can comment on this (CC-ing).
Comment 7 Jesse Glick 2009-07-23 14:56:54 UTC
A clean build is overkill to rebuild the cache. 'ant init' should suffice. (We could even consider having
apisupport.project _write_ to the cache in addition to nbbuild, but this introduces synchronization issues; would need
to take out a file lock. In principle there is already a race condition from two Ant builds running at once, but this we
would consider user error since the NB build system is not thread-safe in general.)

"why whole NB.org sources are scanned instead of just transitive module deps" - well ModuleList.getEntry does try to
directly look up a nb.org module by code name (because it knows that "org.netbeans.modules.foo" must be located in
${nb_all}/foo). The problem is that other code in Evaluator requests a complete module list in the course of registering
the classpaths for a module. You can insert stack traces into ModuleList; I think scanNetBeansOrgStableSources or
getAllEntries{,Soft} is what you are looking for.

Last I remember, one hard problem is modules which use e.g. ${core.startup.dir} somewhere in project.properties;
currently the standard PropertyEvaluator impl does not support lazily reading property values only if and when needed.
(See #155808 for some more on this.) Perhaps we could just drop support for these *.dir properties in apisupport.project
- they would still work in the Ant scripts, but any module which used them in e.g. test.unit.cp.extra would be out of
luck. This style of defining test CPs is anyway long deprecated. Unfortunately these properties are still used more
legitimately in some modules, e.g. ant.browsetask. If loading these props is the _only_ reason a full module scan is
triggered during module CP init, then maybe they could be defined in some other way, e.g. direct inspection of
cluster.properties; but there may be other places where the full module list is requested.
Comment 8 Jesse Glick 2009-08-13 19:15:54 UTC
The performance problem in the thread dump is actually issue #168450, already fixed - nothing to do with the module list.

However reporter may be seeing unrelated performance problems with excessive module list scanning. I have a patch for
that which seems to work well for me, at least for opening projects and working with them normally.

Richard please review. Some calls to getAllEntriesSoft might need to be changed to getAllEntries. For example, if you
open just ant.debugger and expand <this layer in context>, you see only layers from "upstream" modules, which might be
undesirable; probably similar issue with New File wizards. A possibility to consider: remove getAllEntriesSoft (just
have getAllEntries); have getAllEntries use scanNetBeansOrgStableSources if that has not been done yet (convert
lazyNetBeansOrgList into a three-level flag).
Comment 9 Jesse Glick 2009-08-13 19:16:53 UTC
Created attachment 86212 [details]
Start of a patch
Comment 10 Jesse Glick 2009-08-13 19:31:06 UTC
The Evaluator part of the patch I went ahead and did as core-main #797729b2749e.
Comment 11 Jesse Glick 2009-08-13 19:49:43 UTC
A quick analysis of code that asks for a complete or semicomplete nb.org module list:

1. ModuleList.getAllEntriesSoft

1a. SingleModuleProperties.getAllTokens: not very important, just makes it nicer to add a token; can keep but this
should be run asynch.

1b. SingleModuleProperties.reloadModuleListInfo: probably can't remove. Should only be run if showing Properties dialog
though.

1c. SourceRootsSupport.getSourceLocationOfModule: for nb.org modules can probably use a shortcut (e.g.
.../org-netbeans-modules-foo-bar.jar => ${nb_all}/foo.bar).

2. ModuleList.getAllEntries

2a. SuiteCustomizerLibraries.addExtCluster: no idea.

2b. SourceRootsSupport: see 1c.

3. NbPlatform.getModules

3a. LayerUtils.getPlatformJars: probably doesn't matter since normally called on a binary platform.

3b. SuiteProperties: compatibility mode only.

3c. BrandingSupport.init: not sure.

4. NbPlatform.getSortedModules

4a. SuiteCustomizerLibraries: can probably stay as it is; run asynch.

4b. NbPlatformCustomizerModules: fine, intended to show complete list, runs asynch.

5. LayersUtils.getProjectsForNetBeansOrgProject

5a. LayerNode.createClasspath: could probably use something like module.run.classpath instead.

5b. LayerUtils.getEffectiveSystemFileSystem: probably want to keep as it is; run asynch.
Comment 12 Quality Engineering 2009-08-14 06:02:38 UTC
Integrated into 'main-golden', will be available in build *200908140201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/797729b2749e
User: Jesse Glick <jglick@netbeans.org>
Log: Preparation for #169040: avoid ModuleList.getAllEntries{,Soft} where not needed.
Comment 13 rmichalsky 2009-08-18 16:02:40 UTC
Thanks Jesse for taking this issue up, sorry for late reply, I was on vacation. 

Patch and changes pushed so far seem OK to me, there is (currently private) static ModuleList#getClusterProperties(File)
method that returns cached cluster.properties map, it could perhaps be made public and used instead of creating the map
separately in Evaluator.

I agree with removal of getAllEntriesSoft as well, it's IMHO not clear (at least to me) when it is safe to use it anyway.

Re your points:

2a. Used in suites when adding external binary cluster, someone could add platform cluster with sources by mistake
(there is no check), I'll move it to bg thread.

5., 5b. I plan to add layer FS caching for NB.org module projects too in issue #161884, which may help, but probably
won't solve 5a.

5a. Not sure now if module.run.cp contains branding/l10n jars, layer CP could perhaps contain only module classes root +
optional branding/l10n. Probably incompatible change, but I doubt that someone puts bundle entry from some other regular
module into its layer.

Does this list also contain usages up the call hierarchy?
Comment 14 Jesse Glick 2009-08-18 17:23:42 UTC
Yes, ModuleList.getClusterProperties is what Evaluator should be using. I forgot it existed.

getAllEntriesSoft was always a hack. The problem is that (1) there are a lot of modules not in the standard distro, and
you don't really want their layers etc. appearing as "context" when you are working on a module which is in the standard
distro; (2) looking for modules by directory traversal rather than directly reading cluster.properties was historically
slower, though less so now with the flat Hg repo layout. (2) may be obsolete, but (1) is still a real issue and I'm not
sure what to do. Perhaps you can read *.depends properties from cluster.properties and restrict the "universe" of
modules accordingly. For example, from ant.freeform make the ModuleList contain only modules in the platform, harness,
ide, websvccommon, and java clusters. From a module not listed in cluster.properties at all, use the full source scan.

"I doubt that someone puts bundle entry from some other regular module into its layer" - unfortunately a lot of modules
do exactly this. (ValidateLayerConsistencyTest.testLocalizingBundles checks for this, but has too many failures to
enable for now.) Using an icon from another module is even more common. Probably apisupport does not need to support
this poor style, though.

The list was meant to include all direct calls to getAllEntries, with some methods calling it broken out into their own
sections. But of course you should do your own Find Usages and look at the results in more detail.
Comment 15 Jesse Glick 2009-09-09 17:52:28 UTC
*** Issue 170949 has been marked as a duplicate of this issue. ***
Comment 16 Jesse Glick 2009-09-09 17:54:47 UTC
*** Issue 171093 has been marked as a duplicate of this issue. ***
Comment 17 rmichalsky 2009-09-14 14:22:47 UTC
*** Issue 170921 has been marked as a duplicate of this issue. ***
Comment 18 rmichalsky 2009-09-14 14:29:00 UTC
*** Issue 170924 has been marked as a duplicate of this issue. ***
Comment 19 rmichalsky 2009-09-14 14:45:32 UTC
*** Issue 170867 has been marked as a duplicate of this issue. ***
Comment 20 rmichalsky 2009-09-14 14:47:33 UTC
*** Issue 169581 has been marked as a duplicate of this issue. ***
Comment 21 Jesse Glick 2009-09-14 18:22:26 UTC
nbbuild's ModuleListParser should likely also be improved to find dependent modules on demand. I doubt
init-module-all-targets can be fixed, but at least 'ant -f $module/build.xml netbeans' should not need to load a
complete module list.
Comment 22 rmichalsky 2009-09-17 16:16:59 UTC
*** Issue 172490 has been marked as a duplicate of this issue. ***
Comment 23 rmichalsky 2009-09-17 16:57:43 UTC
Recent duplicates reveal another problem: just to make sure that all calls to getAllEntries[Soft] + module list
dependent properties are made from bg threads is not enough, sometimes evaluation of "innocent" property from AWT thread
gets blocked by creation of delegate, which scans module list. There probably has to be two delegates, one for "common"
or module list independent properties and 2nd for module list dependent ones. Only the latter would be re-created when
module list is refreshed. Working on it.

Jesse, re 5a) from your list: so would you agree if I restrict CP for layer nodes the way we've discussed? Icons are not
shown anywhere in <layer in context> and "foreign" bundle keys shown instead of values seem OK to me compared with the
benefits. That would IMHO speed up layerFS views in apisupport considerably too. 
Comment 24 Jesse Glick 2009-09-17 18:02:14 UTC
To 5a - icons _are_ shown in various places, e.g. Services/MIMEResolver. My original suggestion was to use
module.run.classpath, which includes transitive deps of the module, so most "foreign" icons and bundle keys would in
fact still appear, though it would probably also be acceptable to use just resources which can be found inside the same
module.

5a could even perhaps be left untouched, since a week or so ago I changed the icon/label branding for layer nodes to run
asynch. Can still be slower than you might want, but at least it does not block anything else.
Comment 25 Jesse Glick 2009-09-17 18:07:38 UTC
BTW you might be able to simply drop the "module-list-dependency property" optimization in Evaluator if you can (1)
ensure that property evaluation never asks for all entries, (2) make ML not try to look for anything by default.
Comment 26 Jan Lahoda 2009-09-21 19:04:27 UTC
*** Issue 172700 has been marked as a duplicate of this issue. ***
Comment 27 rmichalsky 2009-09-24 15:24:35 UTC
It turned out that there were 3 different culprits that blocked UI:

1) When bg thread was loading module list, the whole evaluator was sometimes blocked. Fixed.
2) All calls to getModuleList from AWT thread were triggered by getting 'cluster' property. Fixed, ${cluster} now
evaluates without ML.
3) Some calls were blocked in native File.lastModified() when loading ML cache. These were reopened and will be handled
separately, there's not much that can be done in apisupport.

I didn't change full ML scan to transitive-deps-only nor I got rid of getAllEntriesSoft() as these would be quite big
changes for Beta and would only speed up bg processing after fix of #1 and #2.

pushed as core-main #e3fbfb3457cd.
Comment 28 Jesse Glick 2009-09-24 15:34:10 UTC
"I didn't change full ML scan to transitive-deps-only nor [did I get] rid of getAllEntriesSoft() as these [...] would
only speed up [background] processing" - fine for 6.8 but there should be a new PERFORMANCE DEFECT opened for this
change. Even if EQ is not blocked, the IDE is still mostly unusable when doing a full module list scan, and this scan is
overkill for many purposes (especially classpath scanning when opening projects).
Comment 29 rmichalsky 2009-09-25 14:28:16 UTC
Ok, I thought about it too. I filed issue #173109, we'll see what time permits.

Comment 30 Quality Engineering 2009-09-26 21:12:54 UTC
Integrated into 'main-golden', will be available in build *200909251401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/e3fbfb3457cd
User: Richard Michalsky <rmichalsky@netbeans.org>
Log: #169040: module list loading no longer blocks evaluator