Skip to content
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

Remove use of unstable box syntax. #18900

Merged
merged 1 commit into from
Oct 16, 2017
Merged

Remove use of unstable box syntax. #18900

merged 1 commit into from
Oct 16, 2017

Conversation

SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Oct 16, 2017

http://www.robohornet.org gives a score of 101.36 on master, and 102.68 with this PR. The latter is slightly better, but probably within noise level. So it looks like this PR does not affect DOM performance.

This is expected since Box::new is defined as:

impl<T> Box<T> {
    #[inline(always)]
    pub fn new(x: T) -> Box<T> {
        box x
    }
}

With inlining, it should compile to the same as box syntax.


This change is Reviewable

http://www.robohornet.org gives a score of 101.36 on master,
and 102.68 with this PR. The latter is slightly better,
but probably within noise level.
So it looks like this PR does not affect DOM performance.

This is expected since `Box::new` is defined as:

```rust
impl<T> Box<T> {
    #[inline(always)]
    pub fn new(x: T) -> Box<T> {
        box x
    }
}
```

With inlining, it should compile to the same as box syntax.
@highfive
Copy link

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/request.rs, components/script/dom/workerlocation.rs, components/script/dom/textencoder.rs, components/script/dom/dompoint.rs, components/script/dom/webglframebuffer.rs and 265 more
  • @fitzgen: components/script/dom/request.rs, components/script/dom/workerlocation.rs, components/script/dom/textencoder.rs, components/script/dom/dompoint.rs, components/script/dom/webglframebuffer.rs and 265 more
  • @emilio: components/script/dom/webglframebuffer.rs, components/script/dom/webglbuffer.rs, components/script/dom/webglshader.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/webglcontextevent.rs and 14 more

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 16, 2017
@SimonSapin
Copy link
Member Author

@bors-servo r=emilio p=1

@bors-servo
Copy link
Contributor

📌 Commit aa15dc2 has been approved by emilio

@highfive highfive assigned emilio and unassigned asajeffrey Oct 16, 2017
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 16, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit aa15dc2 with merge a9022be...

bors-servo pushed a commit that referenced this pull request Oct 16, 2017
Remove use of unstable box syntax.

http://www.robohornet.org gives a score of 101.36 on master, and 102.68 with this PR. The latter is slightly better, but probably within noise level. So it looks like this PR does not affect DOM performance.

This is expected since `Box::new` is defined as:

```rust
impl<T> Box<T> {
    #[inline(always)]
    pub fn new(x: T) -> Box<T> {
        box x
    }
}
```

With inlining, it should compile to the same as box syntax.

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

☀️ 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 a9022be to master...

@bors-servo bors-servo merged commit aa15dc2 into master Oct 16, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 16, 2017
@SimonSapin SimonSapin deleted the box_syntax branch October 16, 2017 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants