Closed Bug 444763 Opened 12 years ago Closed 11 years ago

Use an unshared cache connection instead of a shared one

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 3.1a2

People

(Reporter: sdwilsh, Assigned: sdwilsh)

References

Details

Attachments

(1 file, 1 obsolete file)

In my simple test benchmark, I was seeing a decrease in the number of ticks used for a sample location bar query that would find no results.  The decrease was about 7000 ticks from 90000 ticks to 83000 ticks.  Roughly a 7% win.

The downside to this is if someone were to open up another connection to the database in our process, and does writes, we can see a perf hit.  However, add-ons (or anyone really) shouldn't be doing that.

cc'ing thunder just to make sure weave isn't doing that.
Just a note - the numbers weren't super consistent, but I saw the difference range from 6000 - 10000 ticks.
so, there's one minor issue with this.  Once we are off the shared cache, we can't open a database connection in the tests.  There are three tests that do this:
unit/test_421180.js
unit/test_expiration.js
unit/test_history.js

The last one checks if there's a column in the database, and I think it's safe to remove.  The other two check things that we cannot remove.
Attached patch v1.0 (obsolete) — Splinter Review
mconnor suggested that I just make a testing only interface to work around the issue for the tests.

I made it abstracted enough that we can add more if stuff like this comes up in the future.
Attachment #329093 - Flags: review?(dietrich)
Whiteboard: [has patch][needs review dietrich]
This would effectively disable usage by extensions like SQLiteManager, as well as the kind of exploration done here: http://surfmind.com/muzings/?p=154.

There's potential for complex visualizations and other innovative uses of Places data via extensions that we'll never stay ahead of via the API. We should look at extensions who need to go outside the API, and use their needs as feedback for deciding what API improvements to make.

Basically, I feel like this is a bad tradeoff. We can probably win this perf in other ways - that don't close off the database.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
Whiteboard: [has patch][needs review dietrich] → [has patch]
Another comment: There are not overwhelming problems with Awesomebar performance. If we had a slew of "awesome is slow as molasses" bugs, I'd be far more hawkish here. But we don't.
Attachment #329093 - Flags: review?(dietrich)
Depends on: 449506
We can actually do this without hurting folks because of bug 449506 now
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Whiteboard: [has patch] → [needs new patch]
Target Milestone: Firefox 3.1a1 → Firefox 3.1a2
Attached patch v2.0Splinter Review
Attachment #329093 - Attachment is obsolete: true
Attachment #332634 - Flags: review?(dietrich)
Whiteboard: [needs new patch] → [has patch][needs review dietrich]
Comment on attachment 332634 [details] [diff] [review]
v2.0

r=me
Attachment #332634 - Flags: review?(dietrich) → review+
Whiteboard: [has patch][needs review dietrich] → [has patch][has review][can land]
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/bc658ebc5c77
Status: REOPENED → RESOLVED
Closed: 12 years ago11 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [has patch][has review][can land]
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.