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

Trim some fat from the traversal #17741

Merged
merged 3 commits into from Jul 15, 2017
Merged

Trim some fat from the traversal #17741

merged 3 commits into from Jul 15, 2017
+171 −195

Conversation

@bholley
Copy link
Contributor

bholley commented Jul 15, 2017 •
edited by larsbergstrom

https://bugzilla.mozilla.org/show_bug.cgi?id=1380877

This change is Reviewable
@highfive
Copy link

highfive commented Jul 15, 2017

Heads up! This PR modifies the following files:

  • @canaltinova: components/style/data.rs, components/style/traversal.rs, components/style/parallel.rs, components/style/sequential.rs, components/style/gecko/traversal.rs
  • @emilio: components/style/data.rs, components/style/traversal.rs, components/style/parallel.rs, components/style/sequential.rs, components/style/gecko/traversal.rs
@highfive
Copy link

highfive commented Jul 15, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style code, but no tests are modified. Please consider adding a test!
@bholley
Copy link
Contributor Author

bholley commented Jul 15, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

📌 Commit af6fb88 has been approved by emilio
@highfive highfive assigned emilio and unassigned KiChjang Jul 15, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

⌛ Testing commit af6fb88 with merge 3e6e145...
bors-servo added a commit that referenced this pull request Jul 15, 2017
@bors-servo
Auto merge of #17741 - bholley:trim_traversal_fat, r=emilio
Loading status checks…
3e6e145 
Trim some fat from the traversal

https://bugzilla.mozilla.org/show_bug.cgi?id=1380877
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

💔 Test failed - linux-dev
@bholley bholley force-pushed the bholley:trim_traversal_fat branch from af6fb88 to b29d9cd Jul 15, 2017
@bholley
Copy link
Contributor Author

bholley commented Jul 15, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

📌 Commit b29d9cd has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

⌛ Testing commit b29d9cd with merge fb9ac04...
bors-servo added a commit that referenced this pull request Jul 15, 2017
@bors-servo
Auto merge of #17741 - bholley:trim_traversal_fat, r=emilio
Loading status checks…
fb9ac04 
Trim some fat from the traversal

https://bugzilla.mozilla.org/show_bug.cgi?id=1380877

<!-- 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/17741)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

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

bors-servo commented Jul 15, 2017

☔ The latest upstream changes (presumably #17744) made this pull request unmergeable. Please resolve the merge conflicts.
@bholley bholley force-pushed the bholley:trim_traversal_fat branch from b29d9cd to ffd2504 Jul 15, 2017
bholley added 2 commits Jul 15, 2017
@bholley
Avoid memmoving the large smallvec in Parallel.rs.
2a58c07 
MozReview-Commit-ID: 7nzjMwOmszZ
@bholley
Move clearing of dirty descendants bit closer to the last place it's …
e534ec9 
…needed.

In the next patch, we'll move logic to identify the children for traversal into
preprocess_children (which will be renamed), and the set_dirty_descendants logic
will move along with it. So left as-is, the code here will clobber the flags.

MozReview-Commit-ID: 7ZskKWD4QC3
@bholley bholley force-pushed the bholley:trim_traversal_fat branch from ffd2504 to a89bc23 Jul 15, 2017
@bholley
Copy link
Contributor Author

bholley commented Jul 15, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

📌 Commit a89bc23 has been approved by emilio
@emilio
emilio reviewed Jul 15, 2017
View changes
components/style/traversal.rs Outdated
element.has_animation_only_dirty_descendants()
} else {
element.has_dirty_descendants()
};
if context.shared.traversal_flags.for_animation_only() {

This comment has been minimized.

Sign in to view
@emilio

emilio Jul 15, 2017

Member

nit: You could use flags here.
@bholley
Pass a callback to recalc_style_at to avoid traversing children twice.
Loading status checks…
b7de96e 
MozReview-Commit-ID: DIHXaVNzbFM
@bholley bholley force-pushed the bholley:trim_traversal_fat branch from a89bc23 to b7de96e Jul 15, 2017
@bholley
Copy link
Contributor Author

bholley commented Jul 15, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

📌 Commit b7de96e has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

⌛ Testing commit b7de96e with merge 17f99e2...
bors-servo added a commit that referenced this pull request Jul 15, 2017
@bors-servo
Auto merge of #17741 - bholley:trim_traversal_fat, r=emilio
Loading status checks…
17f99e2 
Trim some fat from the traversal

https://bugzilla.mozilla.org/show_bug.cgi?id=1380877

<!-- 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/17741)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

☀️ Test successful - android, 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: emilio
Pushing 17f99e2 to master...
@bors-servo bors-servo merged commit b7de96e into servo:master Jul 15, 2017
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

@emilio emilio

Assignees

@emilio emilio

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@bholley @highfive @bors-servo @emilio @KiChjang
You can’t perform that action at this time.