Closed Bug 1372794 Opened 2 years ago Closed 2 years ago

Remove the expensive do_GetService() call from nsFormFillController::StopControllingInput()

Categories

(Toolkit :: Form Manager, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This shows up under the react-redux Speedometer test.  The form fill controller gets called from <https://searchfox.org/mozilla-central/rev/d67ef71097da4d1aa344c9d9c672e49a7228e765/toolkit/components/satchel/nsFormFillController.cpp#907> and then we get to this do_GetService() call which shows up in the profile: <https://searchfox.org/mozilla-central/rev/d67ef71097da4d1aa344c9d9c672e49a7228e765/toolkit/components/satchel/nsFormFillController.cpp#1369>

I couldn't measure up a total speedup in points in the total Speedometer benchmark with this fixed, but it's still worth fixing.
Comment on attachment 8877411 [details] [diff] [review]
Cache the form autocomplete service instead of repeatedly getting it in the form fill controller

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

Thanks
Attachment #8877411 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8877411 [details] [diff] [review]
Cache the form autocomplete service instead of repeatedly getting it in the form fill controller

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

::: toolkit/components/satchel/nsFormFillController.cpp
@@ +10,2 @@
>  #include "mozilla/ErrorResult.h"
> +#include "mozilla/Unused.h"

Though is Unused.h used in this patch? I'm not seeing it used but I'm also not an expert in our C++.
(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #3)
> ::: toolkit/components/satchel/nsFormFillController.cpp
> @@ +10,2 @@
> >  #include "mozilla/ErrorResult.h"
> > +#include "mozilla/Unused.h"
> 
> Though is Unused.h used in this patch? I'm not seeing it used but I'm also
> not an expert in our C++.

No you're right, I used it in a previous version and then removed the usage and forgot to remove the #include!
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b084196d6c8
Cache the form autocomplete service instead of repeatedly getting it in the form fill controller; r=MattN
https://hg.mozilla.org/mozilla-central/rev/1b084196d6c8
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.