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 152576 - Do not scan shortcuts folder at startup
Summary: Do not scan shortcuts folder at startup
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Window System (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: Jesse Glick
URL:
Keywords: PERFORMANCE
: 17527 133009 (view as bug list)
Depends on: 61928 170155 170677 172422 179391 180543 196977
Blocks: 143060
  Show dependency tree
 
Reported: 2008-11-06 20:51 UTC by Jesse Glick
Modified: 2011-04-06 13:27 UTC (History)
7 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Proof-of-concept patch (22.25 KB, patch)
2008-11-06 20:54 UTC, Jesse Glick
Details | Diff
Updated patch (applies against trunk sources) (29.14 KB, patch)
2009-07-02 00:37 UTC, Jesse Glick
Details | Diff
Progress (67.21 KB, patch)
2009-07-17 00:42 UTC, Jesse Glick
Details | Diff
Updated patch; everything working that I know of (85.94 KB, patch)
2009-07-30 22:50 UTC, Jesse Glick
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2008-11-06 20:51:59 UTC
Currently we load the entire contents of Shortcuts/ (and Keymaps/NetBeans/ or whatever) during startup, which is slow,
for the sole purpose of creating a reverse action -> keystroke map so we can display accelerators in menu items (and
toolbar button tool tips). This means actions with defined keystrokes are loaded even if they are not displayed in any
menu or toolbar, despite the fact that for these keys the reverse map is never consulted; and even if we delay creation
of menus until they are posted (or toolbars until they are shown), their actions with accelerators will get loaded
eagerly anyway. This is not very smart, and I think we can avoid a complete scan by doing a couple of tricks in core &
openide.

(While changing every module in the IDE to make every action "lazy" using issue #137709 would be helpful as well, a
generic solution would involve fewer changes and would also improve performance of old or third-party modules. Probably
we should do both.)

First, implement NbKeymap.getAction to directly load the right action by looking up the file named by the keystroke.
This should be relatively straightforward.

Second, implement NbKeymap.getKeyStrokesForAction to find the shortcut registration for the action, without loading
unrelated actions. This is a little trickier. We need to know where in the SFS the action was defined. There are two
common cases:

1. A SystemAction is registered using a simple *.instance file (class given by file name, usually, sometimes by
instanceClass attr). It may or may not be in Actions/. The Menu/ and Shortcuts/ definitions are independent (and object
identity is ensured by SharedClassObject).

2. Some other Action is registered using an instance file (of whatever mechanism) in Actions/. Menu/ and Shortcuts/ link
to this definition using *.shadow files to ensure that the same Action instance is loaded.

Since we only care about actions actually being displayed in a menu or toolbar, we can just make MenuBar and Toolbar
keep track of which file was used to define the Action, and note its class name (in case #1) or action pool filename
(case #2). NbKeymap can then report what, if any, keystrokes are also defined using the same class or action pool entry,
and when loading the action we can bind its accelerator property.

I am attaching a fairly straightforward patch which implements this basic logic as a proof of concept. While it shows
the tricks working and most menu items and toolbars in the IDE in fact display working accelerators, it is not
production-quality code:

1. Only the NetBeans keymap profile is supported.

2. Changes in shortcut definitions in the SFS are not listened to.

3. Multi-key shortcuts are not supported. (I don't know much about this system, but since such accelerators cannot be
displayed by Swing anyway AFAIK, we would only need to implement getAction.)

4. Various boundary conditions (null values etc.) are not handled.

5. Other methods in NbKeymap are unimplemented. AFAIK they are not called at runtime anyway (since only
ShortcutAndMenuKeyEventProcessor plus some stuff in openide looks for Keymap in lookup) and we could just return
null/empty values.

6. I am not sure the handling of D- and O- pseudo-modifiers is correct, though it looks to work at least in some cases
(e.g. D-1.shadow for opening the Projects tab).
Comment 1 Jesse Glick 2008-11-06 20:54:10 UTC
Created attachment 73397 [details]
Proof-of-concept patch
Comment 2 Jesse Glick 2008-11-06 20:56:20 UTC
Note that ShortcutsFolder.java is unused in the patch - all the logic is in NbKeymap.
Comment 3 Jesse Glick 2008-11-19 00:09:44 UTC
*** Issue 133009 has been marked as a duplicate of this issue. ***
Comment 4 Antonin Nebuzelsky 2009-03-23 14:52:42 UTC
Assuming this one is a candidate for 6.7 waiver. Jardo?
Comment 5 Jaroslav Tulach 2009-03-27 15:52:12 UTC
I may be wrong, but my guess is that it is a long way from Jesse's patch 
http://www.netbeans.org/nonav/issues/showattachment.cgi/73397/lazy-shortcuts-152576.diff
to industrial strength solution. The tricks with "definingFile", static hashmaps, etc. make me feel quite 
uncomfortable. I definitely do not want to implement this for 6.7.
Comment 6 Jesse Glick 2009-03-27 16:11:57 UTC
I believe the basic approach is workable, though I need to do some research especially on multikey shortcuts. I agree it
should not be attempted for 6.7 at this point - we are too close to the release. (If I had known you were not going to
touch it, I might have worked on it further in November.)

Whether delaying loading of actions with shortcuts will significantly improve startup time is another question. Comments
in issue #133009 indicated 2% could be saved, though we have more modules now. I would recommend that we get rid of
MenuWarmUpTask: in my experience there are a number of menus I rarely use at all, and anyway if and when you first use a
menu it will not take very long to load. Better to devote all the CPU power during startup to reopening editor documents
and scanning CP. With such a change, the improvement from this patch might be a bit more.
Comment 7 Jesse Glick 2009-07-02 00:34:03 UTC
When menu warmup is disabled and toolbars are hidden, starting dev sources built using cluster.config=full (minus
ergonomics) on a warm userdir with a project open and a class open in the editor, -verbose:class shows the updated patch
saving between 130 and 190 out of about 10 class loads, i.e. around 1.6%, mostly action classes or classes clearly
associated with actions. (Time using netbeans.close=true also seems to be down around 2%, though this may not be
statistically significant.) Depending on actual circumstances, you may or may not see all of that benefit.

In other words, probably worth doing, though not likely to yield dramatic results. BTW it is instructive to just start
the IDE (doing nothing but showing a class in a j2seproject in the editor) and look at all the weird modules which load
classes; we could probably achieve substantial startup time increases just by randomly picking such classes and figuring
out how not to load them.
Comment 8 Jesse Glick 2009-07-02 00:37:22 UTC
Created attachment 84275 [details]
Updated patch (applies against trunk sources)
Comment 9 Jesse Glick 2009-07-17 00:42:05 UTC
Created attachment 84869 [details]
Progress
Comment 10 Jesse Glick 2009-07-30 22:23:08 UTC
I think I have got multikey bindings working correctly, though it's hard to be sure, especially since there are so few
of them in the IDE normally.
Comment 11 Jesse Glick 2009-07-30 22:50:57 UTC
Created attachment 85578 [details]
Updated patch; everything working that I know of
Comment 12 Jesse Glick 2009-07-30 22:53:32 UTC
If you have some knowledge of/interest in the esoterica of NbKeymap, please take a look at the last patch. I plan to
commit it Monday, after M1 has been branched.
Comment 13 Jaroslav Tulach 2009-08-03 13:01:50 UTC
Please modify this change in AlwaysEnabledAction to be backward compatible. The following is not:

+        Object o = extractCommonAttribute(map, this, name);
+        // cf. #137709 JG18:
+        return o != null ? o : super.getValue(name);

Please justify why following code cannot remain (other reasons than JG18). Imho the code will work just fine (under 
the assumption the definingFile attribute is provided) without this change:

-        if (Action.ACCELERATOR_KEY.equals(name)) {
-            Keymap map = Lookup.getDefault().lookup(Keymap.class);
-            if (map != null) {
-                KeyStroke[] arr = map.getKeyStrokesForAction(action);
-                return arr.length > 0 ? arr[0] : null;
-            }
-        }

If this is just a matter of your personal preference (e.g. JG18), then keep the current behaviour, please.
Comment 14 Jesse Glick 2009-08-03 15:59:20 UTC
The patch to AEA simply makes getValue return what putValue put into it, if and when putValue is called (which it now
will be). The special treatment of Action.ACCELERATOR_KEY in AEA is not only unnecessary given the rest of the patch - a
relic of the old system of managing accelerators - it will not work (which is why I made this change to begin with).
Comment 15 Jesse Glick 2009-08-03 19:24:20 UTC
Effect using cluster.config=full (minus ergonomics), during second startup on a userdir, passing no other flags (so
leaving toolbars visible and menu warmup on), ignoring a few random-looking reflection classes (such as proxies):

- java.awt.peer.CanvasPeer
- java.text.BreakIterator
- java.text.BreakIterator$1
- java.text.BreakIterator$BreakIteratorCache
- java.text.RuleBasedBreakIterator
- java.text.RuleBasedBreakIterator$1
- java.text.spi.BreakIteratorProvider
- java.util.Observable
- java.util.Observer
- org.netbeans.core.NbKeymap$KeymapAction
- org.netbeans.core.ShortcutsFolder
- org.netbeans.core.ShortcutsFolder$Listener
- org.netbeans.core.actions.NextViewCallbackAction
- org.netbeans.core.actions.PreviousViewCallbackAction
- org.netbeans.core.windows.actions.NextTabAction
- org.netbeans.core.windows.actions.PreviousTabAction
- org.netbeans.modules.db.sql.editor.ui.actions.RunSQLAction
- org.netbeans.modules.db.sql.editor.ui.actions.SQLExecutionBaseAction
- org.netbeans.modules.db.sql.editor.ui.actions.SQLHistoryAction
- org.netbeans.modules.debugger.jpda.ui.debugging.SwitcherTableItem$Activatable
- org.netbeans.modules.form.ComponentContainer
- org.netbeans.modules.form.RADComponent
- org.netbeans.modules.form.RADVisualComponent
- org.netbeans.modules.form.RADVisualContainer
- org.netbeans.modules.form.actions.DuplicateAction
- org.netbeans.modules.form.actions.ReloadAction
- org.netbeans.modules.j2ee.metadata.model.api.MetadataModelAction
- org.netbeans.modules.j2ee.metadata.model.api.MetadataModelException
- org.netbeans.modules.javahelp.HelpAction
- org.netbeans.modules.profiler.ppoints.ui.ToggleProfilingPointAction
- org.netbeans.modules.profiler.ppoints.ui.ToggleProfilingPointAction$ProfilingPointsSwitcher
- org.netbeans.modules.ruby.rubyproject.rake.RakeRunnerAction
- org.netbeans.modules.web.beans.api.model.AmbiguousDependencyException
- org.netbeans.modules.web.beans.api.model.WebBeansModelException
- org.netbeans.modules.web.beans.navigation.actions.InspectInjectablesAtCaretAction
- org.netbeans.modules.web.core.EditQueryStringAction
- org.netbeans.progress.module.CancelAction
- org.openide.NotifyDescriptor$InputLine
- org.openide.awt.Actions$1
- sun.awt.EventQueueDelegate$Delegate
- sun.text.CompactByteArray
- sun.text.SupplementaryCharacterData
- sun.text.resources.BreakIteratorInfo
+ org.netbeans.core.NbKeymap$2
+ org.netbeans.core.NbKeymap$3
+ org.netbeans.core.NbKeymap$Binding
+ org.openide.actions.HeapView$HeapGrowTimer

Again, effects will vary according to the details of what modules you have enabled, whether menu warmup is performed,
which toolbars (if any) are visible, and what exactly you do with the IDE once open.
Comment 16 Jesse Glick 2009-08-03 20:45:31 UTC
core-main #89657b845762
Comment 17 Quality Engineering 2009-08-04 05:47:06 UTC
Integrated into 'main-golden', will be available in build *200908040201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/89657b845762
User: Jesse Glick <jglick@netbeans.org>
Log: Issue #152576: do not load actions solely in order to assign accelerators to menu items.
Comment 18 Jaroslav Tulach 2009-08-04 07:04:00 UTC
*** Issue 17527 has been marked as a duplicate of this issue. ***