1150903 - about:passwords is too slow during filtering

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

See also Bug 1147534. This bug is: it reloads the content on every search keypress. And it seems to do it on the main thread. And each load takes six seconds. 

Regrettably I'm back to the add-on I was using before: it loads everything into a page, never touching the login manager until it really needs to.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

As pointed out in comment 0, we try to filter on each keypress/input event, which is not a good idea. We should create a timer to allow for a 200ms (?) pause before kicking off a filter.

Pulling the items from the loginManager is a sync call, no way around that and likely a waste of time to try any service worker shenanigans for a small (less than 1000) list of passwords. Building the DOM is a main thread activity too. The slow loading in bug 1147534 coupled with the "each keypress" behavior are the first things to fix. Then we see where we are with performance.

Next steps would be looking at the | sort | we do when fetching the list from the loginManager, although this is likely not a big deal.

If we need more complexity we could try:
1. filtering the existing DOM list with each new | _filter | call instead of starting with a fresh full list from the loginManager. This could save a lot of time wasted when building the DOM list, but also has the drawback of causing a DOM reflow on each "removal", so don't jump blindly into this.
2. Cache a full, sorted list of logins so we don't always go back to the loginManager until we know the cache might be stale. The cached list can be updated when we get the "passwordmgr-storage-changed" notification.
2.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: passwords-delay-filter v0.1

This patch uses the common pattern used in Mozilla code to provide a "debouncing" timer for filtering calls. We wait for a 500ms (default for XUL search textboxes) pause during typing to kick off a filter.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #1)

> If we need more complexity we could try:

> 2. Cache a full, sorted list of logins so we don't always go back to the
> loginManager until we know the cache might be stale. The cached list can be
> updated when we get the "passwordmgr-storage-changed" notification.

We already do this.

Comment 4

[Chenxia Liu [:liuche]]

Comment on attachment 8588590 [details] [diff] [review]
passwords-delay-filter v0.1

Review of attachment 8588590 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this is a good perf win for small effort.

Comment 5

[Chenxia Liu [:liuche]]

I'm updating the bug name to reflect that, and we'll see if fixing bug 1147534 is enough; otherwise, we'll spin off other perf bugs.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/fx-team/rev/624e6c8bb11d

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/624e6c8bb11d