Forms and passwords engines: yield back to the main thread during batches of synchronous I/O

RESOLVED FIXED

Status

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

People

(Reporter: philikon, Assigned: philikon)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(fennec2.0+)

Details

(Whiteboard: [sync-perf])

Attachments

(1 attachment, 1 obsolete attachment)

Measurements indicate that form and password batch sizes are too large for mobile.

Updated

7 years ago
tracking-fennec: --- → 2.0+
Blocker needs an owner
Assignee: nobody → philipp
Whiteboard: [sync-perf]
My tests have shown that smaller batch sizes for passwords and forms lead to more fsyncs and thus more blocking events which would be a step back to before bug 630720 and bug 631001.

Larger batch sizes seem to make no difference at all for history. Whereas for forms and passwords it means that we do even larger blocks of synchronous I/O. I think the solution at this point is to keep large batch sizes for lower number of fsyncs but yield back to the main thread in between. This way we will cut events that currently block for several seconds into smaller chunks.
Summary: Tune applyIncomingBatchSizes for history, password, form engines → Forms and passwords engines: yield back to the main thread during batches of synchronous I/O
Created attachment 517471 [details] [diff] [review]
v1

Move SyncEngine._sleep to Store._sleep so that it's easily available to the store objects. Use it after applyIncoming in the passwords and forms engine, as well as some other places where we do synchronous I/O in passwords.
Attachment #517471 - Flags: review?(rnewman)
Comment on attachment 517471 [details] [diff] [review]
v1

Apart from

-DISABLE_TESTS_BUG_604565 = false;
+DISABLE_TESTS_BUG_604565 = true;

which seems to be a local artifact and doesn't apply on my machine, code looks good to me. Tests pass. Good hack!
Attachment #517471 - Flags: review?(rnewman) → review+
Created attachment 517506 [details] [diff] [review]
v1.1

One more carefully placed sleep(0). Also get rid of accidental head_appinfo.js.in change.
Attachment #517471 - Attachment is obsolete: true
Attachment #517506 - Flags: review?(rnewman)
(Assignee)

Updated

7 years ago
Attachment #517506 - Attachment is patch: true
Attachment #517506 - Attachment mime type: application/octet-stream → text/plain
Attachment #517506 - Flags: review?(rnewman) → review+
Landed in fx-sync: https://hg.mozilla.org/services/fx-sync/rev/2e239958c81a
Whiteboard: [sync-perf] → [sync-perf][fixed in fx-sync]
Merged to s-c: http://hg.mozilla.org/services/services-central/rev/eff51222337e
Whiteboard: [sync-perf][fixed in fx-sync] → [sync-perf][fixed in fx-sync][fixed in services]
Merged to m-c: http://hg.mozilla.org/mozilla-central/rev/eff51222337e
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Whiteboard: [sync-perf][fixed in fx-sync][fixed in services] → [sync-perf]
STRs for QA: Fennec should be somewhat more responsive when syncing down lots of form items or passwords. (Fennec 4 final compared to beta 5.)
You need to log in before you can comment on or make changes to this bug.