Optimize `SetProperty` for inline styles #6823

pcwalton · 2015-07-29T02:10:59Z

r? @Ms2ger

pcwalton · 2015-07-29T02:11:06Z

also r? @SimonSapin for the style pieces

highfive · 2015-07-29T02:11:07Z

Warning

These commits modify unsafe code. Please review it carefully!

SimonSapin · 2015-07-29T02:12:18Z

I have some of this already in #6741

metajack · 2015-07-29T16:11:25Z

@SimonSapin can we get yours landed then so this can be rebased?

SimonSapin · 2015-07-29T16:51:16Z

I’m in the middle of addressing review comments on #6741.

Ms2ger · 2015-07-30T11:17:34Z

components/script/dom/element.rs

@@ -89,6 +89,7 @@ use std::ascii::AsciiExt;
 use std::borrow::{Cow, ToOwned};
 use std::cell::{Ref, RefMut};
 use std::default::Default;
+use std::intrinsics;


bors-servo · 2015-07-31T23:08:33Z

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

bors-servo · 2015-08-01T22:54:01Z

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

pcwalton · 2015-09-18T08:45:36Z

Rebased.

r? @Ms2ger


        script: Make `SetProperty` faster by avoiding synthesizing an entire

property declaration block. Wins a few FPS in the particles demo.


        script: Stop copying the document URL.


        script: Compare enum discriminants rather than names when determining

which property to update.


        script: Wrap a long line.

SimonSapin · 2015-09-18T09:03:45Z

components/style/properties.mako.rs

-struct PropertyDeclarationParser<'a, 'b: 'a> {
-    context: &'a ParserContext<'b>,
+pub struct PropertyDeclarationParser<'a, 'b: 'a> {
+    pub context: &'a ParserContext<'b>,


Why this change? It doesn’t seem to be used.

Ah, it’s probably left over from a rebase. As Ms2ger says, the commit can be skipped.

SimonSapin · 2015-09-18T09:05:13Z

components/style/properties.mako.rs

+    #[inline]
+    pub fn id(&self) -> u64 {
+        unsafe {
+            intrinsics::discriminant_value(self)


This uses the same ID for all custom properties. Maybe return Atom instead? (With static atoms for non-custom properties)

Ms2ger · 2015-09-18T09:09:09Z

r- 0dcb42a; it doesn't do anything.
r+ 1bd179e if you revert the changes to fallback_base_url; they will need to be reverted anyway when we implement the about:blank/srcdoc parts.
r? SimonSapin 1699fe8 (if you address my earlier comment about the import)
r+ 668a668

bors-servo · 2015-09-23T21:33:33Z

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

pcwalton · 2015-09-25T01:55:32Z

Split the parts that have r+ into #7736


        Auto merge of #7736 - pcwalton:less-ambitious-optimize-set-property, …

…r=Ms2ger Less ambitious optimize set property Splitting the parts that have r+ out of #6823.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7736)

pcwalton · 2015-11-02T18:12:14Z

Going to go ahead and close this since the important stuff has landed.

highfive added the S-awaiting-review label Jul 29, 2015

metajack added S-blocked-on-external and removed S-awaiting-review labels Jul 29, 2015

metajack assigned Ms2ger Jul 29, 2015

Ms2ger reviewed Jul 30, 2015
View changes

components/script/dom/element.rs

@@ -89,6 +89,7 @@ use std::ascii::AsciiExt;

use std::borrow::{Cow, ToOwned};

use std::cell::{Ref, RefMut};

use std::default::Default;

use std::intrinsics;

This comment has been minimized.

Sign in to view

Copy link

Ms2ger Jul 30, 2015
Contributor

Unused?

metajack added the S-needs-rebase label Jul 31, 2015

pcwalton force-pushed the pcwalton:optimize-set-property branch from 83988df to cdc8996 Aug 1, 2015

highfive added the S-awaiting-review label Aug 1, 2015

SimonSapin removed the S-blocked-on-external label Aug 3, 2015

pcwalton force-pushed the pcwalton:optimize-set-property branch from cdc8996 to 668a668 Sep 18, 2015

pcwalton added S-awaiting-review and removed S-awaiting-review S-needs-rebase labels Sep 18, 2015

pcwalton added 4 commits Jul 29, 2015

script: Make `SetProperty` faster by avoiding synthesizing an entire

0dcb42a

property declaration block. Wins a few FPS in the particles demo.

script: Stop copying the document URL.

1bd179e

script: Compare enum discriminants rather than names when determining

1699fe8

which property to update.

script: Wrap a long line.

Loading status checks…

668a668

SimonSapin reviewed Sep 18, 2015
View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Sep 18, 2015

highfive added S-needs-rebase and removed S-awaiting-merge labels Sep 23, 2015

pcwalton mentioned this pull request Sep 25, 2015

Less ambitious optimize set property #7736

Merged

pcwalton closed this Nov 2, 2015

servo / servo

Optimize `SetProperty` for inline styles #6823

Optimize `SetProperty` for inline styles #6823

pcwalton commented Jul 29, 2015

pcwalton commented Jul 29, 2015

highfive commented Jul 29, 2015

SimonSapin commented Jul 29, 2015

metajack commented Jul 29, 2015

SimonSapin commented Jul 29, 2015

This comment has been minimized.

bors-servo commented Jul 31, 2015

bors-servo commented Aug 1, 2015

pcwalton commented Sep 18, 2015

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Ms2ger commented Sep 18, 2015

bors-servo commented Sep 23, 2015

pcwalton commented Sep 25, 2015

pcwalton commented Nov 2, 2015

servo / servo

Join GitHub today

Optimize `SetProperty` for inline styles #6823

Optimize `SetProperty` for inline styles #6823

Conversation

pcwalton commented Jul 29, 2015

pcwalton commented Jul 29, 2015

highfive commented Jul 29, 2015

SimonSapin commented Jul 29, 2015

metajack commented Jul 29, 2015

SimonSapin commented Jul 29, 2015

This comment has been minimized.

Ms2ger Jul 30, 2015

bors-servo commented Jul 31, 2015

bors-servo commented Aug 1, 2015

pcwalton commented Sep 18, 2015

This comment has been minimized.

SimonSapin Sep 18, 2015

This comment has been minimized.

SimonSapin Sep 18, 2015

This comment has been minimized.

SimonSapin Sep 18, 2015

Ms2ger commented Sep 18, 2015

bors-servo commented Sep 23, 2015

pcwalton commented Sep 25, 2015

pcwalton commented Nov 2, 2015