Skip to content

style: Make Servo use a single thread-pool for layout-related tasks per process.#22487

Merged
bors-servo merged 1 commit intoservo:masterfrom
emilio:single-layout-thread-pool
Dec 23, 2018
Merged

style: Make Servo use a single thread-pool for layout-related tasks per process.#22487
bors-servo merged 1 commit intoservo:masterfrom
emilio:single-layout-thread-pool

Conversation

@emilio
Copy link
Copy Markdown
Member

@emilio emilio commented Dec 17, 2018 •
edited by SimonSapin
Loading

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

This change is Reviewable

@highfive
Copy link
Copy Markdown

highfive commented Dec 17, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/htmliframeelement.rs, components/constellation/pipeline.rs, components/script/dom/windowproxy.rs, components/script/script_thread.rs
  • @cbrewster: components/constellation/pipeline.rs
  • @paulrouget: components/constellation/pipeline.rs
  • @KiChjang: components/script/dom/htmliframeelement.rs, components/script/dom/windowproxy.rs, components/script/script_thread.rs, components/script_traits/lib.rs

@highfive
Copy link
Copy Markdown

highfive commented Dec 17, 2018

warning Warning warning

  • These commits modify style and script code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 17, 2018
@emilio
Copy link
Copy Markdown
Member Author

emilio commented Dec 17, 2018

r? @jdm

@highfive highfive assigned jdm and unassigned ferjm Dec 17, 2018
@emilio emilio mentioned this pull request Dec 17, 2018
Closed
5 tasks
@emilio
Copy link
Copy Markdown
Member Author

emilio commented Dec 17, 2018

@bors-servo try

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 17, 2018

⌛ Trying commit 14d9ac5 with merge f8f1160...

bors-servo pushed a commit that referenced this pull request Dec 17, 2018
Auto merge of #22487 - emilio:single-layout-thread-pool, r=<try>
f8f1160 
style: Make Servo use a single thread-pool for layout-related tasks per process.

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22487)
<!-- Reviewable:end -->
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 18, 2018

💔 Test failed - status-taskcluster

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Dec 18, 2018
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 18, 2018

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

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Dec 18, 2018
@emilio emilio force-pushed the single-layout-thread-pool branch from 14d9ac5 to e9262be Compare December 18, 2018 13:35
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Dec 18, 2018
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 18, 2018

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

@vimpunk
Copy link
Copy Markdown
Contributor

vimpunk commented Dec 18, 2018

Oh you're fast, I also wanted to do this 😄

@jdm jdm removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 21, 2018
@jdm
Copy link
Copy Markdown
Member

jdm commented Dec 22, 2018

@bors-servo r+

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 22, 2018

📌 Commit e9262be has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-needs-rebase There are merge conflict errors. labels Dec 22, 2018
@emilio
style: Make Servo use a single thread-pool for layout-related tasks p…
006e71c 
…er-process.

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (servo#22478), and to share more code with
Gecko, which is always nice.
@emilio emilio force-pushed the single-layout-thread-pool branch from e9262be to 006e71c Compare December 23, 2018 12:01
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Dec 23, 2018
@emilio
Copy link
Copy Markdown
Member Author

emilio commented Dec 23, 2018

@bors-servo r=jdm

  • Just addressed merge conflicts

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 23, 2018

📌 Commit 006e71c has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 23, 2018
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 23, 2018

⌛ Testing commit 006e71c with merge 3934ef7...

bors-servo pushed a commit that referenced this pull request Dec 23, 2018
Auto merge of #22487 - emilio:single-layout-thread-pool, r=jdm
3934ef7 
style: Make Servo use a single thread-pool for layout-related tasks per process.

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22487)
<!-- Reviewable:end -->
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 23, 2018

💔 Test failed - mac-rel-wpt3

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Dec 23, 2018
@jdm
Copy link
Copy Markdown
Member

jdm commented Dec 23, 2018

@bors-servo retry

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 23, 2018

⌛ Testing commit 006e71c with merge b5a10e9...

bors-servo pushed a commit that referenced this pull request Dec 23, 2018
Auto merge of #22487 - emilio:single-layout-thread-pool, r=jdm
b5a10e9 
style: Make Servo use a single thread-pool for layout-related tasks per process.

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22487)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Dec 23, 2018
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Dec 23, 2018

☀️ Test successful - android-mac, arm32, arm64, linux-rel-css, linux-rel-wpt, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, magicleap, status-taskcluster
Approved by: jdm
Pushing b5a10e9 to master...

@bors-servo bors-servo merged commit 006e71c into servo:master Dec 23, 2018
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 23, 2018
@emilio emilio deleted the single-layout-thread-pool branch December 23, 2018 16:17
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jan 7, 2019
@emilio
Bug 1518045 - Make Servo use a single thread-pool for layout-related …
1834c20 
…tasks per-process. r=jdm

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

This cherry-picks servo/servo#22487, with a few minor
fixes to the build that are landing as part of the sync associated to this bug,
and an lsan exception tweak to point to the right module since it's moving.
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Jan 7, 2019
@emilio
Bug 1518045 - Make Servo use a single thread-pool for layout-related …
10ef6f1 
…tasks per-process. r=jdm

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

This cherry-picks servo/servo#22487, with a few minor
fixes to the build that are landing as part of the sync associated to this bug,
and an lsan exception tweak to point to the right module since it's moving.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
@marco-c
Bug 1518045 - Make Servo use a single thread-pool for layout-related …
b2e1321 
…tasks per-process. r=jdm

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

This cherry-picks servo/servo#22487, with a few minor
fixes to the build that are landing as part of the sync associated to this bug,
and an lsan exception tweak to point to the right module since it's moving.

UltraBlame original commit: 4cfc198196e989cbda49033f03879d7f1b53fae7
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
@marco-c
Bug 1518045 - Make Servo use a single thread-pool for layout-related …
ad52d93 
…tasks per-process. r=jdm

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

This cherry-picks servo/servo#22487, with a few minor
fixes to the build that are landing as part of the sync associated to this bug,
and an lsan exception tweak to point to the right module since it's moving.

UltraBlame original commit: 4cfc198196e989cbda49033f03879d7f1b53fae7
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
@marco-c
Bug 1518045 - Make Servo use a single thread-pool for layout-related …
5ac9d9f 
…tasks per-process. r=jdm

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

This cherry-picks servo/servo#22487, with a few minor
fixes to the build that are landing as part of the sync associated to this bug,
and an lsan exception tweak to point to the right module since it's moving.

UltraBlame original commit: 4cfc198196e989cbda49033f03879d7f1b53fae7
bhearsum pushed a commit to mozilla-releng/staging-firefox that referenced this pull request May 1, 2025
@emilio
Bug 1518045 - Make Servo use a single thread-pool for layout-related …
72c583f 
…tasks per-process. r=jdm

Instead of per-document. This also allows to reuse this thread-pool if needed
for other stuff, like parallel CSS parsing (#22478), and to share more code with
Gecko, which is always nice.

This cherry-picks servo/servo#22487, with a few minor
fixes to the build that are landing as part of the sync associated to this bug,
and an lsan exception tweak to point to the right module since it's moving.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdm jdm jdm approved these changes

Assignees

@jdm jdm

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@emilio @highfive @bors-servo @vimpunk @jdm @ferjm