655728 - bookmarks.js::CreateRecord generates too many queries to the database

Status: RESOLVED WORKSFORME

(Firefox :: Sync, defect)

Description

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

This method calls separate APIs to gather all information on an id.
This could be fixed by allowing Places to query for a single node id (something like bug 497607), or by adding a getItemPropertiesForId to the bookmarks service. Or maybe could just wait for a new async API, provided this issue is taken into account.

Comment 1

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

Actually, once bug 633274 is fixed I could add a small transparent improvement, the bookmarks service could store a temporary hash of the last 10 (or such) queried ids and avoid the db for repeated requests to the same id. Would globally help a bunch of add-ons and would be easy/fast to do before we hack a new API.

Comment 2

[Richard Newman [:rnewman]]

(In reply to comment #1)
> Actually, once bug 633274 is fixed I could add a small transparent
> improvement, the bookmarks service could store a temporary hash of the last
> 10 (or such) queried ids and avoid the db for repeated requests to the same
> id.

That sounds like a reasonable hack! Might also ameliorate Bug 655723, no?

Comment 3

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

(In reply to comment #2)
> (In reply to comment #1)
> That sounds like a reasonable hack! Might also ameliorate Bug 655723, no?

no, what I cache includes basic item information, annotations can't be included in this simple hack (there could be any number of them).
Practically with recent changes I'd have a single "getproperties" method and all getters go through it, so caching is easy.

Comment 4

[Lina Butler [:lina]]

This now uses public Places APIs, and the mirror fetches all info in a single query.