Closed Bug 1573305 Opened 1 year ago Closed 1 year ago

Move the bookmarks mirror `hasChanges` check into Rust

Categories

(Firefox :: Sync, task, P1)

task

Tracking

()

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: Lina, Assigned: Lina)

References

Details

Attachments

(1 file)

...So we don't have to duplicate the localItems CTE in Rust and JS.

Plot twist, I don't think we need hasChanges anymore! In the worst case, we'll do three table scans to check if anything changed, then scan the same tables again to build the trees. Even for large trees, merging takes on the order of milliseconds, and doesn't do any I/O. The scans are the bottleneck here.

When a local or remote item changed, we'd potentially scan three tables
(with an expensive LEFT JOIN!) to check if anything changed...then
scan the same tables again to build the local and remote trees. This
check was originally meant to avoid unnecessary merges. However, the
bottleneck isn't merging now; it's reading from the database.

Since the merger has been rewritten in Rust, is synchronous, doesn't
keep a transaction open for the entire merge (see the
total_sync_changes check), and only emits ops for items that actually
changed, it's more efficient to build and merge optimistically, and
bail before applying if nothing changed.

This commit also moves validateLocalRoots into Rust.

Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f4c0860c498
Remove the check for unmerged synced bookmark changes. r=markh
Regressions: 1573738
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
You need to log in before you can comment on or make changes to this bug.