Last Comment Bug 661662 - Speed up the population of about:permissions' sites list
: Speed up the population of about:permissions' sites list
: perf
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 7
Assigned To: Dão Gottwald [:dao]
: Jared Wein [:jaws] (please needinfo? me)
Depends on:
Blocks: 573176
  Show dependency treegraph
Reported: 2011-06-02 13:44 PDT by Dão Gottwald [:dao]
Modified: 2011-06-29 01:13 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (3.25 KB, patch)
2011-06-02 13:44 PDT, Dão Gottwald [:dao]
margaret.leibovic: review+
Details | Diff | Splinter Review

Description User image Dão Gottwald [:dao] 2011-06-02 13:44:24 PDT
Created attachment 536967 [details] [diff] [review]

I have two profiles where I get a slow script warning when opening about:permissions. The initialization is still way too slow with this patch, but at least the slow script warning goes away for me.
Comment 1 User image :Margaret Leibovic 2011-06-02 15:27:55 PDT
Comment on attachment 536967 [details] [diff] [review]

This looks good.

I didn't notice any slowness when testing, even on my pretty old default profile. Did you try commenting out the call to getSitesFromPlaces? I would guess that's the slowest part. Marco had an idea for a less precise but faster query we could use. It's pretty arbitrary that we're choosing to include those top-frecency sites, so maybe we can get rid of that if we come up with a different way to choose which sites we want to show.
Comment 2 User image :Gavin Sharp [email:] 2011-06-02 15:54:44 PDT
I don't think getSitesFromPlaces is the bottleneck, since it has a limit on the number of items it will add, and the query should be pretty fast. The slowness probably comes from just having to build a very large list, if you have many passwords/exceptions.
Comment 3 User image Dão Gottwald [:dao] 2011-06-02 16:21:16 PDT
The unresponsive script warning points to this line:

    let (enumerator = Services.perms.enumerator) {
>>>   while (enumerator.hasMoreElements()) {
        let permission = enumerator.getNext().QueryInterface(Ci.nsIPermission);
        // Only include sites with exceptions set for supported permission types.
        if (this._supportedPermissions.indexOf(permission.type) != -1) {
Comment 4 User image Dão Gottwald [:dao] 2011-06-03 00:07:27 PDT
Comment 5 User image George Carstoiu 2011-06-29 01:08:35 PDT
Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110628 Firefox/7.0a1

Dao, do you have some steps with which you reproduced the issue - I need them in order to verify whether bug was fixed or not.

Comment 6 User image Dão Gottwald [:dao] 2011-06-29 01:13:27 PDT
There's nothing to really verify here, since this is still slow. See bug 665527.

Note You need to log in before you can comment on or make changes to this bug.