Last Comment Bug 522842 - Don't notify during startup until all engines are loaded
: Don't notify during startup until all engines are loaded
Status: RESOLVED FIXED
[ts]
: perf
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 3.7a2
Assigned To: Ryan Flint [:rflint] (ping via IRC for reviews)
:
: Florian Quèze [:florian] [:flo] (PTO until February 27)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-16 20:12 PDT by Ryan Flint [:rflint] (ping via IRC for reviews)
Modified: 2010-02-24 22:15 PST (History)
1 user (show)
rflint: in‑testsuite-
rflint: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.53 KB, patch)
2009-10-16 20:12 PDT, Ryan Flint [:rflint] (ping via IRC for reviews)
no flags Details | Diff | Splinter Review
Patch (1.53 KB, patch)
2010-02-24 19:41 PST, Ryan Flint [:rflint] (ping via IRC for reviews)
gavin.sharp: review+
Details | Diff | Splinter Review

Description User image Ryan Flint [:rflint] (ping via IRC for reviews) 2009-10-16 20:12:15 PDT
Created attachment 406834 [details] [diff] [review]
Patch
Comment 1 User image Ryan Flint [:rflint] (ping via IRC for reviews) 2010-02-24 19:41:47 PST
Created attachment 428863 [details] [diff] [review]
Patch
Comment 2 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2010-02-24 20:00:01 PST
Comment on attachment 428863 [details] [diff] [review]
Patch

Given that the search bar only adds the observer after initializing the search service, this doesn't have much of an affect in practice, right? Apart from the calls to notifyObservers, I guess, but with no observers that's pretty cheap :)
Comment 3 User image Ryan Flint [:rflint] (ping via IRC for reviews) 2010-02-24 20:19:24 PST
Yeah, it's pretty minimal but was enough to make it visible with dtrace, iirc!

http://hg.mozilla.org/mozilla-central/rev/2858cd90d03c
Comment 4 User image Ryan Flint [:rflint] (ping via IRC for reviews) 2010-02-24 21:45:38 PST
This caused most of the browser chrome search tests to timeout - backed out: http://hg.mozilla.org/mozilla-central/rev/59f6be377829
Comment 5 User image Ryan Flint [:rflint] (ping via IRC for reviews) 2010-02-24 22:02:23 PST
derp. http://hg.mozilla.org/mozilla-central/file/2858cd90d03c/toolkit/components/search/nsSearchService.js#l2635
Comment 6 User image Ryan Flint [:rflint] (ping via IRC for reviews) 2010-02-24 22:15:24 PST
Round 2: Fight!

http://hg.mozilla.org/mozilla-central/rev/241ac7137650

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