Closed Bug 459786 Opened 11 years ago Closed 11 years ago

lazily get mDBVisitTo[Visit|URL]Result

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: sdwilsh, Assigned: sdwilsh)

References

Details

Attachments

(1 file)

We only use it in result nodes when it is notified that something is added, so we don't need to create it at startup...
Whiteboard: [needs patch]
at this point would not be better implementing lazy initialization in mozStorage api so all our statements can take advantage of it?
No.  I looked at every statement that we create in nsNavHistory::InitStatements, and many of those will hit us in Tp when they first run (and a lot of them get used in the same patch).
(In reply to comment #1)
> at this point would not be better implementing lazy initialization in
> mozStorage api so all our statements can take advantage of it?

well, you'd want feedback on the statement's validity immediately, right? which would require parsing it...

another solution is to somehow cache parsed statements in the db file itself, so instead of parsing statements each time the db is opened, they're only parsed once, except when we explicitly invalidate the cache. for most users this would occur only when a new version of Firefox is released.

(In reply to comment #2)
> No.  I looked at every statement that we create in
> nsNavHistory::InitStatements, and many of those will hit us in Tp when they
> first run (and a lot of them get used in the same patch).

Tp is calculated over 400 pageloads, so this would likely be a small hit, or maybe even within the range of noise.
(In reply to comment #3)
> well, you'd want feedback on the statement's validity immediately, right? which
> would require parsing it...
This is also true

> another solution is to somehow cache parsed statements in the db file itself,
> so instead of parsing statements each time the db is opened, they're only
> parsed once, except when we explicitly invalidate the cache. for most users
> this would occur only when a new version of Firefox is released.
There was discussion about this once on the sqlite-users mailing list, but I'm not sure whatever came about it.

> Tp is calculated over 400 pageloads, so this would likely be a small hit, or
> maybe even within the range of noise.
Right, but the pageload where all those statements are initialized is going to end up being quite painful.  I think the better user experience is to have that hit during startup, and not when there is UI showing that ends up being non-responsive.  It may not show up in our benchmarks, but it'll still show up to users.
Summary: lazily get mDBVistTo[Visit|URL]Result → lazily get mDBVisitTo[Visit|URL]Result
Attached patch v1.0Splinter Review
Attachment #343123 - Flags: review?(dietrich)
Whiteboard: [needs patch] → [has patch][needs review dietrich]
Attachment #343123 - Flags: review?(dietrich) → review+
Whiteboard: [has patch][needs review dietrich] → [has patch][has review]
http://hg.mozilla.org/mozilla-central/rev/b4f7b47cceb5
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has review]
You need to log in before you can comment on or make changes to this bug.