Last Comment Bug 696179 - search engine "alias" getter caching is broken, results in many unnecessary synchronous SQLite reads when entering text in the location bar
: search engine "alias" getter caching is broken, results in many unnecessary s...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 10
Assigned To: :Gavin Sharp [email: gavin@gavinsharp.com]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-20 12:38 PDT by :Gavin Sharp [email: gavin@gavinsharp.com]
Modified: 2011-10-25 18:01 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.11 KB, patch)
2011-10-20 12:48 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
rflint: review+
Details | Diff | Review
patch (1.89 KB, patch)
2011-10-21 10:14 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
rflint: review+
Details | Diff | Review

Description :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-20 12:38:36 PDT
We iterate over all engines and retrieve their "alias" to compare to the entered text (for triggering search engines using a keyword). Unfortunately the getter for the alias doesn't cache a null result, which means that we end up hitting SQLite for the value multiple times in the common case (engines not having an alias).

(Jeff was seeing this hit the disk every time, which seems like a failure of caching for SQLite, since this DB is very small and should fit in memory. That's a separate issue that needs investigation though.)
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-20 12:48:21 PDT
Created attachment 568481 [details] [diff] [review]
patch
Comment 2 Jeff Muizelaar [:jrmuizel] 2011-10-21 06:28:52 PDT
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #0)
> (Jeff was seeing this hit the disk every time, which seems like a failure of
> caching for SQLite, since this DB is very small and should fit in memory.
> That's a separate issue that needs investigation though.)

It appears as though this was just rereading of the lock page, which is expected. I have however filed a bug on getting rid of that read too: bug 696364.
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-21 10:14:46 PDT
Created attachment 568698 [details] [diff] [review]
patch

I changed my mind about this after thinking about the "alias" getter - I think it's best to have aliases continue to be |null| if they're not set, rather than |""|. So to preserve that behavior I'm just switching to use "undefined" as the uninitialized value.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-25 11:27:38 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cf9011b5494
Comment 5 Ed Morley [:emorley] 2011-10-25 18:01:06 PDT
https://hg.mozilla.org/mozilla-central/rev/3cf9011b5494

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