Send display lists over IPC in multiprocess mode. #6795

pcwalton · 2015-07-27T21:21:06Z

This patch set introduces the --multiprocess (-M) switch. Right now, all it does it cause display lists to be serialized, but eventually it will cause actual processes to be spawned.

r? @metajack

highfive · 2015-07-27T21:21:11Z

Warning

These commits modify unsafe code. Please review it carefully!

metajack · 2015-07-27T21:57:57Z

-S-awaiting-review +S-awaiting-answer

Reviewed 5 of 5 files at r1, 2 of 2 files at r2, 6 of 6 files at r3, 4 of 4 files at r4, 3 of 3 files at r5.
Review status: all files reviewed at latest revision, 7 unresolved discussions, all commit checks successful.

^{components/compositing/pipeline.rs, line 197 [r3] (raw file):}
Not sure why you changed let _ = to drop(). The original reason for it was so it was clear we were ignoring the return value. I don't think there is a hidden drop here we are trying to make visible.

^{components/compositing/pipeline.rs, line 202 [r3] (raw file):}
Same as above.

^{components/compositing/pipeline.rs, line 291 [r3] (raw file):}
Are these layout_to_paint things used anywhere? I can't seem to find where.

^{components/gfx/paint_task.rs, line 206 [r3] (raw file):}
Shouldn't this be layout_to_paint_handle?

^{components/gfx/paint_task.rs, line 207 [r3] (raw file):}
Ditto above.

^{components/gfx/platform/macos/font_template.rs, line 72 [r5] (raw file):}
Why is this necessary now? Or was it a bug that this didn't have it originally?

^{components/util/ipc.rs, line 32 [r1] (raw file):}
Throwing away errors seems wrong. Can you give some explanation for this? Does IPC not have any error semantics on send?sx

Comments from the review on Reviewable.io

pcwalton · 2015-07-27T22:15:23Z

Review status: all files reviewed at latest revision, 7 unresolved discussions, all commit checks successful.

^{components/compositing/pipeline.rs, line 293 [r3] (raw file):}
Yes, they're used in the invocations of LayoutTaskFactory::create() and PaintTask::create() in the two methods below.

^{components/gfx/platform/macos/font_template.rs, line 72 [r5] (raw file):}
It should have been there before. We need the mutex so that if two threads try to instantiate the CachedCTFont at the same time they won't interfere with one another.

^{components/util/ipc.rs, line 32 [r1] (raw file):}
IPC senders' error codes are OS-specific and different from OS to OS. At the moment ipc-channel does not attempt to abstract over Mach vs. POSIX error codes, as Servo doesn't try to deal with errors in sending on a fine-grained level anyway.

This doesn't actually throw away the error—it just throws away the error description.

Comments from the review on Reviewable.io

pcwalton · 2015-07-27T22:44:25Z

Review comments addressed. r? @metajack

metajack · 2015-07-27T22:49:13Z

@bors-servo r+

-S-awaiting-answer -S-awaiting-review +S-awaiting-merge

Reviewed 2 of 2 files at r6.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

Comments from the review on Reviewable.io

bors-servo · 2015-07-27T22:49:13Z

📌 Commit 4233857 has been approved by metajack

bors-servo · 2015-07-29T00:24:28Z

⌛ Testing commit 4233857 with merge 8ebd958...


        Auto merge of #6795 - pcwalton:display-list-e10s-fixes, r=metajack

@metajack

Send display lists over IPC in multiprocess mode. This patch set introduces the `--multiprocess` (`-M`) switch. Right now, all it does it cause display lists to be serialized, but eventually it will cause actual processes to be spawned. r? @metajack  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6795)

bors-servo · 2015-07-29T01:01:39Z

💔 Test failed - mac3

bors-servo · 2015-07-29T01:04:05Z

📌 Commit 4233857 has been approved by pcwalton

pcwalton · 2015-07-29T01:04:15Z

@bors-servo: retry

#6819, #6820, #6821

bors-servo · 2015-07-29T02:05:08Z

⌛ Testing commit 4233857 with merge 82145d9...


        Auto merge of #6795 - pcwalton:display-list-e10s-fixes, r=pcwalton

@metajack

Send display lists over IPC in multiprocess mode. This patch set introduces the `--multiprocess` (`-M`) switch. Right now, all it does it cause display lists to be serialized, but eventually it will cause actual processes to be spawned. r? @metajack  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6795)

bors-servo · 2015-07-29T02:40:57Z

💔 Test failed - mac3

jdm · 2015-07-29T03:16:46Z

@pcwalton I'm not convinced that these are just intermittent failures that are suddenly appearing for the first time:

/css21_dev/html4/absolute-replaced-height-026.htm
-------------------------------------------------
TIMEOUT expected FAIL [Parent]
/css21_dev/html4/absolute-replaced-height-025.htm
-------------------------------------------------
TIMEOUT expected FAIL [Parent]
/css21_dev/html4/float-replaced-height-004.htm
----------------------------------------------
TIMEOUT expected FAIL [Parent]
/css21_dev/html4/inline-replaced-height-005.htm
-----------------------------------------------
TIMEOUT [Parent]

A bunch of similar replaced-height CSS tests failed on the last round too.

bors-servo · 2015-07-29T09:19:30Z

☔ The latest upstream changes (presumably #6804) made this pull request unmergeable. Please resolve the merge conflicts.

jdm · 2015-07-30T20:48:18Z

/css21_dev/html4/absolute-replaced-height-026.htm
-------------------------------------------------
TIMEOUT expected FAIL [Parent]
/css21_dev/html4/absolute-replaced-height-033.htm
-------------------------------------------------
TIMEOUT expected FAIL [Parent]
/css21_dev/html4/block-replaced-height-005.htm
----------------------------------------------
TIMEOUT expected FAIL [Parent]

No more retries allowed until at least one line of code is changed :)

bors-servo · 2015-07-30T22:30:57Z

☔ The latest upstream changes (presumably #6740) made this pull request unmergeable. Please resolve the merge conflicts.


        util: Add a multiprocess command-line option and implement

`OptionalIpcSender<T>`. `OptionalIpcSender<T>`dynamically switches between in-process and out-of-process communication depending on whether multiprocess mode is enabled. The multiprocess command-line switch doesn't actually turn on multiprocess mode yet, but it does control the behavior of `OptionalIpcSender<T>`.


        util: Remove the old broken `SerializableLinkedList` code in favor of

serde's native implementation.


        gfx: Split the paint messages into layout and chrome messages, and make

the layout messages serializable.


        compositing: Use an `OptionalIpcSender` for communication between the

layout and paint tasks.


        gfx: Instantiate the `CTFont` corresponding to a Mac font on demand.

This avoids panics in multiprocess mode.


        Update `rust-core-text`

pcwalton · 2015-07-31T15:43:30Z

@bors-servo: r=glennw

bors-servo · 2015-07-31T15:43:31Z

📌 Commit 17ead87 has been approved by glennw

bors-servo · 2015-07-31T15:43:42Z

⌛ Testing commit 17ead87 with merge c4480b5...


        Auto merge of #6795 - pcwalton:display-list-e10s-fixes, r=glennw

@metajack

Send display lists over IPC in multiprocess mode. This patch set introduces the `--multiprocess` (`-M`) switch. Right now, all it does it cause display lists to be serialized, but eventually it will cause actual processes to be spawned. r? @metajack  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6795)

pcwalton · 2015-07-31T15:57:03Z

(I landed a fix for what I believe to be the problem here)

bors-servo · 2015-07-31T16:47:48Z

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

highfive added the S-awaiting-review label Jul 27, 2015

metajack added S-awaiting-answer and removed S-awaiting-review labels Jul 27, 2015

pcwalton force-pushed the pcwalton:display-list-e10s-fixes branch from 601dca7 to 4233857 Jul 27, 2015

highfive added the S-awaiting-review label Jul 27, 2015

metajack added S-awaiting-merge and removed S-awaiting-answer S-awaiting-review labels Jul 27, 2015

jdm added S-tests-failed and removed S-awaiting-merge labels Jul 29, 2015

metajack self-assigned this Jul 29, 2015

metajack added the S-needs-rebase label Jul 29, 2015

pcwalton force-pushed the pcwalton:display-list-e10s-fixes branch from 4233857 to 516d642 Jul 30, 2015

highfive removed the S-tests-failed label Jul 30, 2015

jdm removed the S-awaiting-merge label Jul 30, 2015

metajack added the S-needs-rebase label Jul 30, 2015

pcwalton force-pushed the pcwalton:display-list-e10s-fixes branch from 516d642 to 3f09a25 Jul 31, 2015

highfive added S-awaiting-review and removed S-tests-failed labels Jul 31, 2015

pcwalton force-pushed the pcwalton:display-list-e10s-fixes branch from 3f09a25 to e8a8b1d Jul 31, 2015

pcwalton added 6 commits Jul 27, 2015

util: Remove the old broken `SerializableLinkedList` code in favor of

f041e1a

serde's native implementation.

gfx: Split the paint messages into layout and chrome messages, and make

3be5bbb

the layout messages serializable.

compositing: Use an `OptionalIpcSender` for communication between the

1ff7a51

layout and paint tasks.

gfx: Instantiate the `CTFont` corresponding to a Mac font on demand.

e9ec928

This avoids panics in multiprocess mode.

Update `rust-core-text`

Loading status checks…

17ead87

pcwalton force-pushed the pcwalton:display-list-e10s-fixes branch from e8a8b1d to 17ead87 Jul 31, 2015

jdm added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Jul 31, 2015

bors-servo merged commit 17ead87 into servo:master Jul 31, 2015
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

bors-servo mentioned this pull request Jul 31, 2015

script: Make the resource task communication use IPC channels. #6586

Merged

pcwalton deleted the pcwalton:display-list-e10s-fixes branch Jul 31, 2015

This was referenced Aug 3, 2015

Intermittent timeout in /css21_dev/html4/block-replaced-height-005.htm #6821

Closed

Intermittent timeout in /css21_dev/html4/absolute-replaced-height-033.htm #6820

Closed

Intermittent timeout in /css21_dev/html4/absolute-replaced-height-012.htm #6819

Closed

servo / servo

Send display lists over IPC in multiprocess mode. #6795

Send display lists over IPC in multiprocess mode. #6795

pcwalton commented Jul 27, 2015

highfive commented Jul 27, 2015

metajack commented Jul 27, 2015

pcwalton commented Jul 27, 2015

pcwalton commented Jul 27, 2015

metajack commented Jul 27, 2015

bors-servo commented Jul 27, 2015

bors-servo commented Jul 29, 2015

bors-servo commented Jul 29, 2015

bors-servo commented Jul 29, 2015

pcwalton commented Jul 29, 2015

bors-servo commented Jul 29, 2015

bors-servo commented Jul 29, 2015

jdm commented Jul 29, 2015

bors-servo commented Jul 29, 2015

jdm commented Jul 30, 2015

bors-servo commented Jul 30, 2015

pcwalton commented Jul 31, 2015

bors-servo commented Jul 31, 2015

bors-servo commented Jul 31, 2015

pcwalton commented Jul 31, 2015

bors-servo commented Jul 31, 2015

servo / servo

Join GitHub today

Send display lists over IPC in multiprocess mode. #6795

Send display lists over IPC in multiprocess mode. #6795

Conversation

pcwalton commented Jul 27, 2015

highfive commented Jul 27, 2015

metajack commented Jul 27, 2015

pcwalton commented Jul 27, 2015

pcwalton commented Jul 27, 2015

metajack commented Jul 27, 2015

bors-servo commented Jul 27, 2015

bors-servo commented Jul 29, 2015

bors-servo commented Jul 29, 2015

bors-servo commented Jul 29, 2015

pcwalton commented Jul 29, 2015

bors-servo commented Jul 29, 2015

bors-servo commented Jul 29, 2015

jdm commented Jul 29, 2015

bors-servo commented Jul 29, 2015

jdm commented Jul 30, 2015

bors-servo commented Jul 30, 2015

pcwalton commented Jul 31, 2015

bors-servo commented Jul 31, 2015

bors-servo commented Jul 31, 2015

pcwalton commented Jul 31, 2015

bors-servo commented Jul 31, 2015