Introduce task queues, and throttling performance timeline tasksource #21388

gterzian · 2018-08-12T11:10:15Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #19997 (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because _____

This change is

highfive · 2018-08-12T11:10:19Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/script_thread.rs, components/script/lib.rs, components/script/task_queue.rs
@KiChjang: components/script/script_thread.rs, components/script/lib.rs, components/script/task_queue.rs

highfive · 2018-08-12T11:10:21Z

Warning

These commits include an empty title element (<title></title>). Consider adding appropriate metadata.
These commits modify unsafe code. Please review it carefully!

gterzian · 2018-08-12T11:13:30Z

@jdm this is sort of a first draft on the "task-queue" concept, let me know what you think...

gterzian · 2018-08-12T14:09:17Z

@bors-servo try

bors-servo · 2018-08-12T14:09:25Z

⌛ Trying commit 9b886e6 with merge 551a6fc...


        Auto merge of #21388 - gterzian:introduce_task_queues, r=<try>

Introduce task queues, and throttling performance timeline tasksource  ---  - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #19997 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21388)

bors-servo · 2018-08-12T15:06:34Z

☀️ Test successful - android, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True

gterzian · 2018-08-13T11:02:37Z

Last commit is an attempt at #20723, with one caveat, which is that we'll warn! also if the event-loop is simply idle, not just when it's blocked. It is however significantly simpler than using an actual queued task to measure responsiveness...

gterzian · 2018-08-14T13:21:26Z

I'm looking into making this TaskQueue a bit more generic, so that it could be used for the worker event-loops as well(whose execution is currently 'one task per iteration', so will require some changes to 'batch' task per iteration like the script-thread does, so that we can have a concept of a 'busy' event-loop)...

gterzian · 2018-08-15T00:39:02Z

components/script/dom/dedicatedworkerglobalscope.rs

+    fn recv_tasks(&self) -> Vec<WorkerScriptMsg> {
+        self.receiver.try_iter().map(|msg|{
+            let (linked_worker, script_msg) = msg;
+            let _ar = AutoWorkerReset::new(&*self.dedicated_scope, linked_worker);


I think this is actually not going to work, since we should only do the AutoWorkerReset 'right before' handling a particular event...

gterzian · 2018-08-15T22:22:06Z

@jdm This one took me quite far down the rabbit hole, and it is finally ready for review.

Main problems encountered:

CommonScriptMsg::Task is hidden inside different structures of messages for each event-loop, so had to define a trait of operations of how to get it 'in and out' of the messages the event-loops expect.
The worker event-loop doesn't have the luxury of receiving messages from the constellation and other parallel components, so it would actually block on the select in the case that there are throttled tasks ready for processing, but there are no new messages coming in. So I had to add a way to wake-up the event-loop in such a case.
The TrustedWorkerAddress used by the dedicated worker was really a major headache, because it is basically a piece of data that needs to be passed along with the task, and the other event-loops don't have something similar. So QeueudTask has an Option<TrustedWorkerAddress> as a result... I also replaced the tuple (TrustedWorkerAddress, WorkerScriptMsg) with a variant of a new DedicatedWorkerScriptMsg, which is simply easier to work with.
On the way, I removed the generic params of (Trusted<T>, CommonScriptMsg) from the SendableWorkerScriptChan because they weren't actually being used, and they would make the above very hard to actually do, so they now just expect a DedicatedWorkerScriptMsg...

gterzian · 2018-08-16T09:05:54Z

@bors-servo try

bors-servo · 2018-08-16T09:06:01Z

⌛ Trying commit 07375b7 with merge ab43f9a...

jdm · 2018-08-30T16:48:50Z

Just waiting on squashing into the two commits now.

jdm · 2018-08-30T16:49:01Z

@bors-servo delegate=gterzian

bors-servo · 2018-08-30T16:49:02Z

✌️ @gterzian can now approve this pull request


        introduce task-queues, and throttling the performance-timeline task-s…

…ource, in script and worker threads. queue


        introduce a generic worker event-loop

gterzian · 2018-08-30T18:20:31Z

@bors-servo r=jdm

bors-servo · 2018-08-30T18:20:32Z

📌 Commit 029715a has been approved by jdm

gterzian · 2018-08-30T18:21:09Z

@bors-servo r+

bors-servo · 2018-08-30T18:21:09Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: #21523

bors-servo · 2018-08-30T18:21:10Z

📌 Commit 029715a has been approved by gterzian

gterzian · 2018-08-30T18:22:00Z

@bors-servo r=jdm (though it didn't trigger the build first time, but that's not the case it seems)

bors-servo · 2018-08-30T18:22:00Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: #21523

bors-servo · 2018-08-30T18:22:01Z

📌 Commit 029715a has been approved by jdm

gterzian · 2018-08-30T18:28:44Z

either this or #21325 will require a rebase anyhow...

bors-servo · 2018-08-30T18:56:28Z

⌛ Testing commit 029715a with merge 1ee3dee...


        Auto merge of #21388 - gterzian:introduce_task_queues, r=jdm

Introduce task queues, and throttling performance timeline tasksource  ---  - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [ ] These changes fix #19997 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21388)

bors-servo · 2018-08-30T19:51:56Z

💔 Test failed - mac-rel-css1

jdm · 2018-08-30T21:07:26Z

@bors-servo retry

#20734

bors-servo · 2018-08-30T21:08:04Z

⚡ Previous build results for android, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev are reusable. Rebuilding only mac-rel-css1...

bors-servo · 2018-08-30T21:46:08Z

☀️ Test successful - android, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing 1ee3dee to master...

highfive assigned gw3583 Aug 12, 2018

highfive added the S-awaiting-review label Aug 12, 2018

gterzian force-pushed the gterzian:introduce_task_queues branch from 71f096c to e91271c Aug 12, 2018

gterzian mentioned this pull request Aug 12, 2018

Replace mpsc with crossbeam-channel #21325

Merged

gterzian force-pushed the gterzian:introduce_task_queues branch 2 times, most recently from dd5170a to 9b886e6 Aug 12, 2018

gterzian force-pushed the gterzian:introduce_task_queues branch 2 times, most recently from e0b521a to 184c69c Aug 13, 2018

gterzian force-pushed the gterzian:introduce_task_queues branch from 8eb1ad9 to c9235d8 Aug 13, 2018

jdm assigned jdm and unassigned gw3583 Aug 14, 2018

gterzian reviewed Aug 15, 2018

View changes

gterzian force-pushed the gterzian:introduce_task_queues branch 5 times, most recently from a4a62ad to 5a7bb16 Aug 15, 2018

gterzian force-pushed the gterzian:introduce_task_queues branch from 5a7bb16 to 07375b7 Aug 15, 2018

highfive added the S-awaiting-review label Aug 30, 2018

jdm removed the S-awaiting-review label Aug 30, 2018

gterzian added 2 commits Aug 10, 2018

introduce task-queues, and throttling the performance-timeline task-s…

ca6306c

…ource, in script and worker threads. queue

introduce a generic worker event-loop

Loading status checks…

029715a

gterzian force-pushed the gterzian:introduce_task_queues branch from 2b13994 to 029715a Aug 30, 2018

highfive added the S-awaiting-review label Aug 30, 2018

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-squash labels Aug 30, 2018

gterzian mentioned this pull request Aug 31, 2018

Intermittent failure in /_mozilla/mozilla/task_queue_throttling.any.worker.html #21552

Open

gterzian deleted the gterzian:introduce_task_queues branch Jan 7, 2020

servo / servo

Introduce task queues, and throttling performance timeline tasksource #21388

Introduce task queues, and throttling performance timeline tasksource #21388

gterzian commented Aug 12, 2018 •

edited

highfive commented Aug 12, 2018

highfive commented Aug 12, 2018

gterzian commented Aug 12, 2018

gterzian commented Aug 12, 2018

bors-servo commented Aug 12, 2018

bors-servo commented Aug 12, 2018

gterzian commented Aug 13, 2018 •

edited

gterzian commented Aug 14, 2018 •

edited

This comment has been minimized.

gterzian commented Aug 15, 2018

gterzian commented Aug 16, 2018

bors-servo commented Aug 16, 2018

jdm commented Aug 30, 2018

jdm commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

jdm commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

servo / servo

Join GitHub today

Introduce task queues, and throttling performance timeline tasksource #21388

Introduce task queues, and throttling performance timeline tasksource #21388

Conversation

gterzian commented Aug 12, 2018 • edited

highfive commented Aug 12, 2018

highfive commented Aug 12, 2018

gterzian commented Aug 12, 2018

gterzian commented Aug 12, 2018

bors-servo commented Aug 12, 2018

bors-servo commented Aug 12, 2018

gterzian commented Aug 13, 2018 • edited

gterzian commented Aug 14, 2018 • edited

This comment has been minimized.

gterzian Aug 15, 2018

gterzian commented Aug 15, 2018

gterzian commented Aug 16, 2018

bors-servo commented Aug 16, 2018

jdm commented Aug 30, 2018

jdm commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

jdm commented Aug 30, 2018

bors-servo commented Aug 30, 2018

bors-servo commented Aug 30, 2018

gterzian commented Aug 12, 2018 •

edited

gterzian commented Aug 13, 2018 •

edited

gterzian commented Aug 14, 2018 •

edited