Closed
Bug 1421701
Opened 7 years ago
Closed 7 years ago
Look into chunking NotifyManyVisitsObservers rather than sending all visits at once
Categories
(Firefox :: Migration, enhancement, P3)
Firefox
Migration
Tracking
()
RESOLVED
FIXED
Firefox 59
| Tracking | Status | |
|---|---|---|
| firefox59 | --- | fixed |
People
(Reporter: dthayer, Assigned: dthayer)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
Bug 1418443 replaced NotifyVisitObservers with NotifyManyVisitsObservers, which reduces overall time taken by the main thread, but results in all of that time being taken at once. If we chunk the runnables into groups of, say, 100 visits, we might get the same reduction in overall time, but not take it all at once, thus reducing jank.
| Comment hidden (mozreview-request) |
|
Assignee | |
Updated•7 years ago
|
Assignee: nobody → dothayer
Status: NEW → ASSIGNED
|
||
Comment 2•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934310 [details]
Bug 1421701 - Chunk visits when notifying main thread
https://reviewboard.mozilla.org/r/205228/#review213308
::: toolkit/components/places/History.cpp:1092
(Diff revision 1)
> NS_ENSURE_SUCCESS(rv2, rv2);
> }
> }
> NS_ENSURE_SUCCESS(rv, rv);
>
> + int32_t numRemaining = mPlaces.Length() - (i + 1);
can be moved inside the if (shouldChunkNotifications) body
::: toolkit/components/places/History.cpp:1095
(Diff revision 1)
> NS_ENSURE_SUCCESS(rv, rv);
>
> + int32_t numRemaining = mPlaces.Length() - (i + 1);
> + if (shouldChunkNotifications) {
> + notificationChunk.AppendElement(place);
> + if (notificationChunk.Length() > NOTIFY_VISITS_CHUNK_SIZE ||
why ">", it sounds like this will notify 101 entries?
Attachment #8934310 -
Flags: review?(mak77)
| Comment hidden (mozreview-request) |
|
|
||
Comment 4•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934310 [details]
Bug 1421701 - Chunk visits when notifying main thread
https://reviewboard.mozilla.org/r/205228/#review213438
Thank you!
Attachment #8934310 -
Flags: review?(mak77) → review+
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/15a13cdc34d5
Chunk visits when notifying main thread r=mak
|
||
Comment 6•7 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in
before you can comment on or make changes to this bug.
Description
•