Closed Bug 468705 Opened 17 years ago Closed 17 years ago

cache the result of _getSyncTableStatement in nsDBFlush

Categories

(Toolkit :: Places, defect, P3)

Product:
Toolkit
Component:
Places
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.2a1

People

(Reporter: mak, Assigned: sdwilsh)

References

Details

(Keywords: fixed1.9.1, perf)

Attachments

(1 file, 3 obsolete files)

branch patch
3.73 KB, patch
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
Since the statement does not change we could cache it instead of generating it every time.
I talked with taras about this, and because we regenerate these statements every time, this actually hurts mobile. Therefore, I'm nominating this for a P2 blocker.
Flags: blocking1.9.1?
Attached patch v1.0 (obsolete) — — Splinter Review
This should do the trick, but I'm asserting in storage. Until check-interactive works, I really cannot do anything about this...
Assignee: nobody → sdwilsh
Status: NEW → ASSIGNED
Mobile's not releasing on 1.9.1 and is back working on trunk, so wanted not blocking.
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Depends on: 472004
Target Milestone: --- → mozilla1.9.1
this'll still help in firefox, would be nice to get this for 3.1.
Keywords: perf
Priority: -- → P3
duh - I know why it was crashing. We finalize the statements, but we can't do that when we cache them!
Attached patch v1.1 (obsolete) — — Splinter Review
I can be so stupid sometimes...
Attachment #352226 - Attachment is obsolete: true
Attachment #361906 - Flags: review?(mak77)
Whiteboard: [needs review mak]
shouldn't we finalize the cached statements after the last sync on shutdown?
(In reply to comment #7) > shouldn't we finalize the cached statements after the last sync on shutdown? Yup - realized that last night as I was trying to fall asleep. I'll put a patch up with that change in a bit.
Comment on attachment 361906 [details] [diff] [review] v1.1 r=me with finalized statements
Attachment #361906 - Flags: review?(mak77) → review+
Attached patch v1.2 (obsolete) — — Splinter Review
With finalizing. I'll request approval on this once I land it on mozilla-central since this is a low-risk change that gets more work off of the main thread.
Attachment #361906 - Attachment is obsolete: true
Whiteboard: [needs review mak] → [can land]
http://hg.mozilla.org/mozilla-central/rev/57e905485264
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Attachment #362014 - Flags: approval1.9.1?
Depends on: 478948
This caused bug 478948.
Attachment #362014 - Flags: approval1.9.1?
Comment on attachment 362014 [details] [diff] [review] v1.2 This caused a regression, so killing approval request.
Depends on: 479391
Attached patch branch patch — — Splinter Review
This patch is for branch. It combines attachment 36201 [details] [diff] [review] and attachment 363340 [details] [diff] [review] from bug 478948 (regression bug, caused by this bug).
Attachment #362014 - Attachment is obsolete: true
Attachment #364606 - Flags: approval1.9.1?
Comment on attachment 364606 [details] [diff] [review] branch patch a191=beltzner
Attachment #364606 - Flags: approval1.9.1? → approval1.9.1+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c320a12b4b86
Keywords: fixed1.9.1
Target Milestone: mozilla1.9.1 → mozilla1.9.2a1
Note: 1.9.1 check-in rolled in the patch in bug 478948 (as noted in comment 14)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: