audit mozStorage usage in Weave

RESOLVED FIXED in 1.3b3

Status

Cloud Services
Firefox Sync: Backend
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mconnor, Assigned: Mardak)

Tracking

unspecified
1.3b3
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
blocking-weave1.3 +

Firefox Tracking Flags

(Not tracked)

Details

Comment hidden (empty)
(Reporter)

Updated

8 years ago
Target Milestone: --- → 1.3
(Reporter)

Updated

8 years ago
Whiteboard: [final]
(Assignee)

Updated

8 years ago
Whiteboard: [final]
First up: history.js

OK, so selecting from *_view is slow.  Very slow.  It doesn't use an indexes, so don't do it.  Ever.  You'll have to do the joins manually, but I bet mak can help there.

You are using step on mozIStorageStatement, which is deprecated.  It'll be removed probably after 1.9.3.  But really, you should be using executeAsync for all of this.  If you were using the places API, I'd deal with the sync I/O, but you are already rolling your own statements.

I bet these comments apply to all the files.
forms.js

The query you have in getAllEntries is going to be nothing short of very slow since you have a bunch of subselect statements.

Are you really blindly adding the guid column to the form manager table when a statement fails to compile?  There are several better ways to do that (including getting it added in core).
No comments on bookmarks.js, but you should file a bug to have someone on the places team (mak?) look over your API usage.
fx-setup.js

Note that places doesn't keep track of X days of history anymore, so having weave sync Y days of history seems a bit odd.
Audit done.  Someone needs to file bugs based on my feedback.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Blocks: 562378
(Reporter)

Updated

8 years ago
Assignee: sdwilsh → edilee
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

8 years ago
Blocks: 562379
(Assignee)

Updated

8 years ago
Blocks: 562381
(Assignee)

Updated

8 years ago
Blocks: 506402, 559163
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: 1.3 → 1.3b3
(In reply to comment #3)
> Are you really blindly adding the guid column to the form manager table when a
> statement fails to compile?  There are several better ways to do that
> (including getting it added in core).

It has been added to core, but only very recently (bug 506402). Dolske was consulted on what Weave was doing before it happened. Though you're right that it could probably have a more explicit check for the column instead of just adding the it on failure.
You need to log in before you can comment on or make changes to this bug.