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

Avoid transform copies where not necessary #2942

Merged
merged 2 commits into from Aug 1, 2018

Conversation

Projects
None yet
3 participants
@kvark
Member

kvark commented Jul 31, 2018

This should be harmless. Just using references and Cow where we did copies before. Those copies are sometimes difficult to track down in the profiler because they just make the functions heavier and not all the time manifest in separate system move calls.
cc @nical


This change is Reviewable

@kvark kvark requested a review from gw3583 Jul 31, 2018

@gw3583

gw3583 approved these changes Jul 31, 2018

@gw3583

This comment has been minimized.

Show comment
Hide comment
@gw3583

gw3583 Jul 31, 2018

Contributor

This looks good to me. It might be worth a try run, just in case, since some of those changes when detecting transform kind vs. preserves_2d etc can be subtle - but I'll leave that to your judgment :) r=me.

Contributor

gw3583 commented Jul 31, 2018

This looks good to me. It might be worth a try run, just in case, since some of those changes when detecting transform kind vs. preserves_2d etc can be subtle - but I'll leave that to your judgment :) r=me.

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 1, 2018

Contributor

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

Contributor

bors-servo commented Aug 1, 2018

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

@kvark

This comment has been minimized.

Show comment
Hide comment
Member

kvark commented Aug 1, 2018

@kvark

This comment has been minimized.

Show comment
Hide comment
@kvark

kvark Aug 1, 2018

Member

A few intermittents, no blockers found.
@bors-servo r=gw3583

Member

kvark commented Aug 1, 2018

A few intermittents, no blockers found.
@bors-servo r=gw3583

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 1, 2018

Contributor

📌 Commit a855aa3 has been approved by gw3583

Contributor

bors-servo commented Aug 1, 2018

📌 Commit a855aa3 has been approved by gw3583

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 1, 2018

Contributor

⌛️ Testing commit a855aa3 with merge b790123...

Contributor

bors-servo commented Aug 1, 2018

⌛️ Testing commit a855aa3 with merge b790123...

bors-servo added a commit that referenced this pull request Aug 1, 2018

Auto merge of #2942 - kvark:nocopy, r=gw3583
Avoid transform copies where not necessary

This should be harmless. Just using references and `Cow` where we did copies before. Those copies are sometimes difficult to track down in the profiler because they just make the functions heavier and not all the time manifest in separate system move calls.
cc @nical

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

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 1, 2018

Contributor

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw3583
Pushing b790123 to master...

Contributor

bors-servo commented Aug 1, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw3583
Pushing b790123 to master...

@bors-servo bors-servo merged commit a855aa3 into servo:master Aug 1, 2018

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

@kvark kvark deleted the kvark:nocopy branch Aug 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment