Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upSkip rule node which contains only inherited properties for rule cache #19696
Conversation
highfive
commented
Jan 5, 2018
|
Heads up! This PR modifies the following files:
|
highfive
commented
Jan 5, 2018
|
|
|
r? @emilio |
|
Try pushes for performance comparison:
|
|
r=me if Talos / try is not mad at this. |
components/style/rule_cache.rs
Outdated
| } | ||
| } | ||
| StyleSource::None => {} | ||
| StyleSource::Style(_) => { break; } |
This comment has been minimized.
This comment has been minimized.
components/style/rule_cache.rs
Outdated
| @@ -102,7 +134,8 @@ impl RuleCache { | |||
| return None; | |||
| } | |||
|
|
|||
| let rules = builder_with_early_props.rules.as_ref()?; | |||
| let rules = builder_with_early_props.rules.as_ref(); | |||
| let rules = RuleCache::get_rule_node_for_cache(guards, rules)?; | |||
This comment has been minimized.
This comment has been minimized.
components/style/rule_cache.rs
Outdated
| /// rule, nor containing any declaration of reset property. We don't | ||
| /// skip style rule so that we don't need to walk a long way in the | ||
| /// worst case. Skipping declarations rule nodes should be enough | ||
| /// to address common cases that rule cache fails to do reasonable |
This comment has been minimized.
This comment has been minimized.
emilio
Jan 5, 2018
Member
You mean s/rule cache/style sharing cache? The rule cache should hopefully get a decent sharing now :)
This comment has been minimized.
This comment has been minimized.
upsuper
Jan 5, 2018
Author
Member
I mean, the common case that rule cache failed to do without this...
This comment has been minimized.
This comment has been minimized.
|
@bors-servo r=emilio |
|
|
bors-servo
added a commit
that referenced
this pull request
Jan 5, 2018
Skip rule node which contains only inherited properties for rule cache This is one possible fix for [bug 1427681](https://bugzilla.mozilla.org/show_bug.cgi?id=1427681) which tries to skip some rule nodes when using rule cache. Try push for correctness: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74e3941e2cfc5fba4bce839f2518af8a5a8b7411 It doesn't really show much memory saving on awsy. It only shows several KB save on fresh start memory. But since conceptually it's simple, I guess it's worth taking. <!-- 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/19696) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
bors-servo
added a commit
that referenced
this pull request
Jan 5, 2018
Skip rule node which contains only inherited properties for rule cache This is one possible fix for [bug 1427681](https://bugzilla.mozilla.org/show_bug.cgi?id=1427681) which tries to skip some rule nodes when using rule cache. Try push for correctness: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74e3941e2cfc5fba4bce839f2518af8a5a8b7411 It doesn't really show much memory saving on awsy. It only shows several KB save on fresh start memory. But since conceptually it's simple, I guess it's worth taking. <!-- 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/19696) <!-- Reviewable:end -->
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
upsuper commentedJan 5, 2018
•
edited by SimonSapin
This is one possible fix for bug 1427681 which tries to skip some rule nodes when using rule cache.
Try push for correctness: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74e3941e2cfc5fba4bce839f2518af8a5a8b7411
It doesn't really show much memory saving on awsy. It only shows several KB save on fresh start memory. But since conceptually it's simple, I guess it's worth taking.
This change is