Closed Bug 580228 Opened 11 years ago Closed 11 years ago

Relax timed history expiration


(Toolkit :: Places, defect)

Not set



Tracking Status
blocking2.0 --- final+


(Reporter: mak, Assigned: mak)




(1 file, 1 obsolete file)

This patch is relaxing timed expiration a bit:
- history pages limit is checked separately, if below the limit don't run visits statements, that are more expensive than the simple check itself.
- remove from timed expiration any statement for rare orphans like item annotations.
- restore stopping timed expiration on idle, I wrongly disabled it in a previous patch. We should always observe idle but expire on idle only if history is dirty.
- use the larger timer if history status is unknown, so for example the first expiration will happen 9 minutes after start up, rather than 3 minutes after it, there is already enough stuff happening soon after startup (livemarks updates for example).
- avoid excluding orphan place: uris from expiration, since all of them are bookmarks (less work for sqlite)
I fear that the fact Storage is putting multiple async statements in an immediate transaction by default is giving us locking headaches. expiration can run also 10 async statements in that batch, even if they are pretty fast since limited to a few results the sum could be a not ignorable time, I could evaluate to split them in chunks in a follow-up if we discover it's needed, or also executeAsync them one by one, could maybe hurt fsyncs though but at least they are on the async thread.
Attached patch patch v1.0 (obsolete) — Splinter Review
Attachment #458643 - Flags: review?(sdwilsh)
ohr, I forgot to ask blocking, this helps expiration reducing work that is done on a timer to the bare minimum needed, oter work continues to be done on idle or shutdown based on needs.
Also stops timed expiration when an idle starts (useful on mobile devices or laptops to preserve batteries).
Due to how storage locks table reducing timed expiration work can help reducing  lock contentions and time where tables are unaccessible.
blocking2.0: --- → ?
Blocks: 563538
Depends on: 580240
Comment on attachment 458643 [details] [diff] [review]
patch v1.0

Please capitalize 'count' in the queries.  We tend to put function names in all caps, so it should be COUNT(*).  Also, I think you can do COUNT(1) in that query to make it a bit faster.

Attachment #458643 - Flags: review?(sdwilsh) → review+
Attached patch patch v1.1Splinter Review
capitalized COUNT, not changing to COUNT(1) because SQLite optimizes COUNT(*).
Attachment #458643 - Attachment is obsolete: true
blocking2.0: ? → final+
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b3
You need to log in before you can comment on or make changes to this bug.