Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faster display list transfer #9947

Merged
merged 2 commits into from Mar 23, 2016
Merged

Faster display list transfer #9947

merged 2 commits into from Mar 23, 2016

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Mar 10, 2016

This series of commits improves performance of display list construction in browser.html by about 3x when using WebRender.

It requires servo/webrender_traits#18 and servo/webrender#231.

Anyone should feel free to review if they have time; I'll ask someone in particular once those two upstream commits land.

cc @paulrouget

This change is Review on Reviewable
@highfive
Copy link

highfive commented Mar 10, 2016

warning Warning warning

  • These commits modify gfx and layout code, but no tests are modified. Please consider adding a test!
@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 11, 2016 

/home/travis/build/servo/servo/components/layout/layout_thread.rs:958:25: 958:50 error: this function takes 6 parameters but 8 parameters were supplied [E0061]

/home/travis/build/servo/servo/components/layout/layout_thread.rs:958                     api.set_root_stacking_context(sc_id,

                                                                                              ^~~~~~~~~~~~~~~~~~~~~~~~~

/home/travis/build/servo/servo/components/layout/layout_thread.rs:958:25: 958:50 help: run `rustc --explain E0061` to see a detailed explanation

/home/travis/build/servo/servo/components/layout/webrender_helpers.rs:561:22: 561:46 error: no method named `next_stacking_context_id` found for type `&mut webrender_traits::api::RenderApi` in the current scope

/home/travis/build/servo/servo/components/layout/webrender_helpers.rs:561         let id = api.next_stacking_context_id();

                                                                                               ^~~~~~~~~~~~~~~~~~~~~~~~

/home/travis/build/servo/servo/components/layout/webrender_helpers.rs:571:22: 571:42 error: no method named `next_display_list_id` found for type `&mut webrender_traits::api::RenderApi` in the current scope

/home/travis/build/servo/servo/components/layout/webrender_helpers.rs:571         let id = api.next_display_list_id();

                                                                                               ^~~~~~~~~~~~~~~~~~~~

/home/travis/build/servo/servo/components/layout/webrender_helpers.rs:573:63: 573:73 error: no method named `descriptor` found for type `webrender_traits::display_list::BuiltDisplayList` in the current scope

/home/travis/build/servo/servo/components/layout/webrender_helpers.rs:573                                                  display_list.descriptor().has_stacking_contexts;

                                                                                                                                        ^~~~~~~~~~

error: aborting due to 4 previous errors

Could not compile `layout`.
@jdm
Copy link
Member

jdm commented Mar 21, 2016

The webrender-related PRs have merged successfully.
@pcwalton pcwalton force-pushed the pcwalton:faster-display-list-transfer branch from fca8cc8 to 4ee343e Mar 21, 2016
@pcwalton
Copy link
Contributor Author

pcwalton commented Mar 21, 2016

Updated to include the WR updates.

r? @glennw
@highfive highfive assigned glennw and unassigned mbrubeck Mar 21, 2016
@pcwalton pcwalton mentioned this pull request Mar 21, 2016
Open
@glennw
Copy link
Member

glennw commented Mar 21, 2016

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Mar 21, 2016

📌 Commit 4ee343e has been approved by glennw
@bors-servo
Copy link
Contributor

bors-servo commented Mar 21, 2016

⌛ Testing commit 4ee343e with merge 0aececa...
bors-servo added a commit that referenced this pull request Mar 21, 2016
@bors-servo
Auto merge of #9947 - pcwalton:faster-display-list-transfer, r=glennw
Loading status checks…
0aececa 
Faster display list transfer

This series of commits improves performance of display list construction in browser.html by about 3x when using WebRender.

It requires servo/webrender_traits#18 and servo/webrender#231.

Anyone should feel free to review if they have time; I'll ask someone in particular once those two upstream commits land.

cc @paulrouget

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

bors-servo commented Mar 21, 2016

💔 Test failed - mac-rel-wpt
@bors-servo
Copy link
Contributor

bors-servo commented Mar 21, 2016

☔ The latest upstream changes (presumably #10021) made this pull request unmergeable. Please resolve the merge conflicts.
@pcwalton pcwalton force-pushed the pcwalton:faster-display-list-transfer branch from 4ee343e to e348f93 Mar 21, 2016
components/layout/context.rs Outdated
// See if the image is already available
let result = self.shared.image_cache_thread.find_image(url.clone(),
use_placeholder);
let result = self.shared.image_cache_thread.find_image(url.clone(), UsePlaceholder::No);

This comment has been minimized.

Sign in to view
@jdm

jdm Mar 22, 2016

Member

Why the switch to UsePlaceholder::No here?
@glennw
Copy link
Member

glennw commented Mar 22, 2016

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Mar 22, 2016

📌 Commit 0cc0a13 has been approved by glennw
@glennw
Copy link
Member

glennw commented Mar 22, 2016

@bors-servo r-
@pcwalton pcwalton force-pushed the pcwalton:faster-display-list-transfer branch from 0cc0a13 to 0006c19 Mar 22, 2016
@pcwalton
Copy link
Contributor Author

pcwalton commented Mar 22, 2016

@jdm @glennw Added use_placeholder back. (I don't think it affects anything either way, but anyway.)

r? @glennw
@pcwalton
gfx: Allow images to be shipped to the WebRender thread without shipping
Loading status checks…
0006c19 
over the data as well.

WebRender doesn't need the data, as it acquires it separately.

About a 50%-100% improvement in display list building time on
browser.html.
@glennw
Copy link
Member

glennw commented Mar 22, 2016

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Mar 22, 2016

📌 Commit 0006c19 has been approved by glennw
@bors-servo
Copy link
Contributor

bors-servo commented Mar 22, 2016

⌛ Testing commit 0006c19 with merge 54b9112...
bors-servo added a commit that referenced this pull request Mar 22, 2016
@bors-servo
Auto merge of #9947 - pcwalton:faster-display-list-transfer, r=glennw
Loading status checks…
54b9112 
Faster display list transfer

This series of commits improves performance of display list construction in browser.html by about 3x when using WebRender.

It requires servo/webrender_traits#18 and servo/webrender#231.

Anyone should feel free to review if they have time; I'll ask someone in particular once those two upstream commits land.

cc @paulrouget

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

bors-servo commented Mar 22, 2016

💔 Test failed - android
@pcwalton
Copy link
Contributor Author

pcwalton commented Mar 22, 2016

cc @larsbergstrom
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Mar 23, 2016

@bors-servo retry

  • infra
@bors-servo
Copy link
Contributor

bors-servo commented Mar 23, 2016

⌛ Testing commit 0006c19 with merge 187ca44...
bors-servo added a commit that referenced this pull request Mar 23, 2016
@bors-servo
Auto merge of #9947 - pcwalton:faster-display-list-transfer, r=glennw
Loading status checks…
187ca44 
Faster display list transfer

This series of commits improves performance of display list construction in browser.html by about 3x when using WebRender.

It requires servo/webrender_traits#18 and servo/webrender#231.

Anyone should feel free to review if they have time; I'll ask someone in particular once those two upstream commits land.

cc @paulrouget

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

bors-servo commented Mar 23, 2016

☀️ Test successful - android, arm32, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor
@bors-servo bors-servo merged commit 0006c19 into servo:master Mar 23, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@glennw glennw

Labels
S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
@pcwalton @highfive @Ms2ger @jdm @glennw @bors-servo @larsbergstrom @mbrubeck @paulrouget
You can’t perform that action at this time.