459786 - lazily get mDBVisitTo[Visit|URL]Result

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Shawn Wilsher :sdwilsh]

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...

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

at this point would not be better implementing lazy initialization in mozStorage api so all our statements can take advantage of it?

Comment 2

[Shawn Wilsher :sdwilsh]

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).

Comment 3

[Dietrich Ayala (:dietrich)]

(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.

Comment 4

[Shawn Wilsher :sdwilsh]

(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.

Comment 5

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

Comment 6

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/b4f7b47cceb5