LoginManagerContent.jsm shows up in the page load profiles, consider rewriting in C++?
Categories
(Toolkit :: Password Manager, enhancement, P2)
Tracking
()
Performance Impact | medium |
People
(Reporter: smaug, Unassigned)
References
Details
(Keywords: perf:pageload, Whiteboard: [fxperf:p3])
It is pretty common for random .jsms to show up during page load.
jsms also tend to use more memory than what they would use if implemented natively.
It is both loading .jsms and also executing the js which take some time.
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Bug 1519306 will avoid loading any login manager JSMs unless the page has a password field. The profile in question is for a page which has a password field though. Bug 1149500 which is on our roadmap for the next month may help for background tabs.
Coincidentally I was just thinking this morning about whether it makes sense to move some hot parts to native code. The one downside is that longer term we'd like to share logic with non-Gecko (FxiOS and non-Fx webext.) environments so that would get more complicated. At least with current resourcing and plans, I don't think re-writing the whole JSM in C++ would be great but we can consider moving hot parts to native code.
The main issue is that we try to autofill login field when they appear so we need to do some amount of work when loading a document with them.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 2•6 years ago
|
||
I filed bug 1527828 for moving the address bar insecure login field calculations (from pageshow and DOMFormHasPassword) to C++ as they were shown in the profile from comment 0 and are kinda independent of the rest of password manager.
On a related note, the insecure login field console warnings from InsecurePasswordUtils.jsm could also be consolidated with the address bar indicator logic and we could avoid loading that JSM on page load too. There is also telemetry being accumulated on every page load in InsecurePasswordUtils which could have a perf impact.
I also have ideas for other areas but will have more context on that soon when I start working on bug 1287202 again.
Comment 3•5 years ago
|
||
Is this better now? A bunch of the referenced bugs are fixed, or duped to bugs that were fixed - but it's not clear if some of the reasons by which they would help with this bug still apply given how they were duped (e.g. switching to an actor model wouldn't necessarily mean we don't load things unless a pw field is present).
Comment 4•5 years ago
|
||
It should be but the bug was never super clear on what is acceptable. Given that we run this code outside Gecko on iOS and want to do that more for a non-Firefox web extension, I don't think moving to C++ makes sense.
Updated•3 years ago
|
Description
•