Last Comment Bug 661662 - Speed up the population of about:permissions' sites list
: Speed up the population of about:permissions' sites list
Status: RESOLVED FIXED
: perf
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 7
Assigned To: Dão Gottwald [:dao]
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

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

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 :Margaret Leibovic 2011-06-02 15:27:55 PDT
Comment on attachment 536967 [details] [diff] [review]
patch

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 :Gavin Sharp [email: gavin@gavinsharp.com] 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 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) {
          this.addHost(permission.host);
        }
      }
    }
Comment 4 Dão Gottwald [:dao] 2011-06-03 00:07:27 PDT
http://hg.mozilla.org/mozilla-central/rev/328bc4f3183a
Comment 5 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.

Thanks!
Comment 6 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.