Open Bug 483121 Opened 11 years ago Updated 4 years ago
closing the engine manager dialog after multiple changes is slow with many windows open
(2:40:11 AM) ccaygill: gavin: how many instances of "getVisibleEngines: getting all visible engines" would be expected on search plugin addition / removal? (2:41:47 AM) ccaygill: (I see 4 groups of 4 for adding and 1+4 for removal ?) (2:41:48 AM) gavin: ccaygill: hmm... none? (2:42:08 AM) gavin: ccaygill: I see one call on engine dialog initialization, and one for search bar initialization (2:42:19 AM) gavin: ccaygill: oh, wait (2:43:10 AM) gavin: ccaygill: the search bar potentially retrieves the list once each time an engine is "modified" (2:43:34 AM) gavin: ccaygill: so if you "one for each open window", I suppose (2:44:03 AM) ccaygill: hmm... still seems to be more than that - let me test with a single window... (2:48:59 AM) ccaygill: gavin: that seems to explain why each 'group' was of 4 but there are still 4 entries for addition... once when loaded with default favicon then again with correct icon then seemingly once after cache invalidating and once at the end? (removal makes sense though... once for building manage search engines and once to reload when the deletions are made) (2:49:45 AM) gavin: hmm, yeah (2:49:51 AM) gavin: that is not very well optimized (2:49:59 AM) gavin: http://mxr.mozilla.org/mozilla-central/source/browser/components/search/content/search.xml#212 (2:50:14 AM) ccaygill: removing multiple engines at the same time seems to rebuild the list once for each removal as well (2:50:26 AM) gavin: we clear the cache unconditionally for all notifications, and then immediately call rebuildPopup which retrieves the engines again (2:50:54 AM) ccaygill: (which would explain why the dialogue is a bit slow to clear when I delete say 10 plugins with 4 windows open... hmm) (2:51:14 AM) gavin: we could probably avoid clearing the cache for some notifications and/or batch notifications to avoiding more than one retrieval
The engine manager dialog no longer exists, but most of the problems described in comment 0 are related either to the search service or the searchbar binding, so this bug is probably still relevant.
You need to log in before you can comment on or make changes to this bug.