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 17527 - ShortcutsFolder is too eager
Summary: ShortcutsFolder is too eager
Status: VERIFIED DUPLICATE of bug 152576
Alias: None
Product: platform
Classification: Unclassified
Component: -- Other -- (show other bugs)
Version: 3.x
Hardware: PC Linux
: P3 blocker (vote)
Assignee: issues@platform
URL:
Keywords: PERFORMANCE
Depends on: 13540
Blocks: 17597 26580 133009
  Show dependency tree
 
Reported: 2001-11-12 09:11 UTC by Jesse Glick
Modified: 2009-08-04 16:01 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2001-11-12 09:11:07 UTC
All ShortcutsFolder needs to do is provide a Map<KeyStroke,Action>
from the contents of the Shortcuts/ folder. It reads in the entire
folder and loads and resolves every action in it at startup. If some
module binds e.g. Ctrl-Super-F17 to RefactorEntityBeanAction, and the
user never types this key combination, this is wasting a fair amount
of classloading (since loading a SystemAction involves loading and
resolving a new class). It ought to be possible to produce a smart Map
implementation that only loads an action when Map.get() is actually
called (normally when that key is really pressed). This should be
feasible since the map keys are just data object names and it is
relatively quick to check for the existence of a data object with a
given name in the Shortcuts/ folder. Marking lower-priority since most
keyboard shortcuts are more commonly-used actions like Execute that
are often already present in the main menu bar anyway (which probably
has to load everything eagerly).
Comment 1 Jan Chalupa 2001-11-27 11:51:41 UTC
Target milestone -> 3.3.1.
Comment 2 Jan Chalupa 2001-11-27 11:55:09 UTC
Target milestone -> 3.3.1.
Comment 3 Jan Chalupa 2002-01-11 14:00:18 UTC
Target milestone -> 3.4
Comment 4 Jan Chalupa 2002-01-11 14:04:18 UTC
Target milestone -> 3.4
Comment 5 Jan Chalupa 2002-01-11 14:05:56 UTC
Target milestone -> 3.4
Comment 6 Jan Chalupa 2002-01-11 14:09:52 UTC
Target milestone -> 3.4
Comment 7 Marek Grummich 2002-07-22 08:30:47 UTC
Target milestone was changed from '3.4' to TBD.
Comment 8 Marek Grummich 2002-07-22 08:35:02 UTC
Target milestone was changed from '3.4' to TBD.
Comment 9 David Simonek 2002-07-23 13:58:47 UTC
taking...
Comment 10 Jesse Glick 2002-09-10 17:32:13 UTC
Should probably be considered higher priority, right? If we want to
reduce startup time and memory footprint, this may make a measurable
difference. AFAIK no one has measured the impact of this problem
specifically.
Comment 11 Petr Nejedly 2002-09-11 15:26:34 UTC
I've just measured it. If I forced synchronous parsing of the folder,
it took ~800ms (~3%) on my machine. (Otherwise it is asynchronous
so the initialization will actually happen later in the time slice
usually attributed to ToolbarPool (heh, and I was wondering what is
taking 800ms on TP parsing, now it shows only 200ms for TP)

Theoretically if may be possible to implement the lazy map but here
are some of the problems (besides Keymap being *strange* interface):

*) There is also backward mapping (Action->KeyStroke[]) exercised
 during the startup. It is used for binding tooltips to ToolbarButtons
 and side-comments to MenuItems. Solution: MenuItems are not loaded
 during the startup and tooltips can be evaluated on demand.

*) The name of the file can be (deprecated but... compatibility) in
 the form of KeyStroke[ClassName].instance. That means at least
 an index of the Shortcuts folder will have to be initialized.

On the other hand, 800ms is nice...
Comment 12 Jesse Glick 2002-09-11 16:42:06 UTC
Re. #1 - yuck, but I like your proposed workaround.

Re. #2 - I don't understand what the purpose of this is. Also it
directly contradicts issue #15926. What is the "index"?
Comment 13 Petr Nejedly 2002-09-11 17:12:15 UTC
Ad 2)
There may be modules still having class name in their binding file
If you're going to lazily lookup action for, say, C-F1,
it isn't enough to look for file named C-F1 but rather for C-F1*
which FS doesn't support. The good thing is that
iterate(shortcutsFO.getChildren()) takes around 40ms on my machine.
index = prescanned content of the folder to allow more sophisticated
lookups, like C-F1*

(I understood it that you'd like to implement a map with get()
implemented like:
public Object get(Object key) {
	FileObject fo = folder.getFileObject((String)key);
	DataObject dobj = DataObject.find(fo);
	InstanceCookie ic = dobj.getCookie(InstanceCookie.class);
	return ic.instanceCreate();
})

To the issue 15926: Adding about a hundred of folders may not be the
best thing but who knows...
Comment 14 Jesse Glick 2002-09-12 16:39:00 UTC
Re. #2 - fine, I completely misunderstood your earlier comment. I
thought you meant we should *replace* existing C-F1.instance with
C-F1[com-foo-Action].instance for some reason!

Re. adding a folder for each key binding - it's probably a little
overhead if we are using an XML layer cache. I hope that by making a
binary layer cache there will be no overhead for files/folders in
layers unless you are actually traversing them - in which case having
a folder Shortcuts/F1/ and one or more files in it would be the same
cost as the existing Shortcuts/F1.instance.

BTW the "fast" version of the map (FileObject.getFileObject(binding))
*would* be possible if we chose a new folder name for shortcuts (e.g.
"KeyBindings"), and #15926 were done: then KeyBindings/$binding/ would
have all the new-style bindings. You would still have to browse
through Shortcuts/ at startup, but since the whole folder would be
deprecated, it would be empty or nearly empty, so cheap to look
through. Just an idea.
Comment 15 Jesse Glick 2003-01-10 19:43:13 UTC
Problem #1 (reverse mapping) may force us to mark this WONTFIX now -
since we have a warmup task that will probably load every menu item.
It sounds pretty hard to find which key bindings are used for an
action without loading all the actions.
Comment 16 David Simonek 2005-04-28 16:52:10 UTC
Jesse and Petr, I don't have a clue what you are talking about here :-) Pretty
please resolve this issue for me somehow. Thanks.
Comment 17 Jesse Glick 2005-04-28 17:11:07 UTC
It's probably WONTFIX as I indicated in my last comment; I don't see how to do
it correctly and still include accelerator info in menu items.
Comment 18 David Simonek 2005-04-28 17:15:01 UTC
OK, so I'm brave and closing this as wontfix :-)
Comment 19 _ rkubacki 2005-05-02 12:48:50 UTC
Probably we need a bigger rewritte here. Currently when run synchronously it
takes ~750ms on my P4/2GHz notebook (added
ShortcutsFolder.waitShortcutsFinished). The growing number of shortcuts (approx.
100 now) shows that it does not scale well.
Comment 20 David Simonek 2005-05-02 16:50:19 UTC
OK, reopening... but guys could one of you please take care of this? I don't
plan to spend any time on this, sorry.
Comment 21 David Simonek 2008-10-16 16:02:08 UTC
Passing to general component...
Comment 22 Jaroslav Tulach 2008-10-24 08:46:47 UTC
Actually, during investigation of issue 133009 I found that simple lazy map is not enough:
http://www.netbeans.org/nonav/issues/showattachment.cgi/67930/X.diff
The problem is that when an action is shown in Menu, its shortuct is supposed to be visible there. This requires 
processing of the whole Shortcuts folder in current system.

The only solution I see is to have proxies just like suggested in issue 13540. Only, rather than mangling with layers, 
we want some clever annotations, as introduced in issue 150804, and sketched in 
http://www.netbeans.org/nonav/issues/showattachment.cgi/72515/AdvanturesWithActionRegistrationProcessor.diff
Comment 23 Jaroslav Tulach 2009-08-04 07:04:01 UTC
Shortcuts folder is no longer read on start due to fix of issue 152576

*** This issue has been marked as a duplicate of 152576 ***
Comment 24 Jesse Glick 2009-08-04 16:01:26 UTC
Right, this was the same issue, I just forgot I had ever filed it. :-) Backward mapping solved with a small amount of
cooperation from the code which loads menus and toolbars from disk.