Add preferences to enable/disable the JITs #10325

ghost · 2016-04-01T00:48:02Z

When creating a runtime (script.rs), we check prefs and then enable or
disable the JITs (Baseline and Ion) accordingly.

Closes #10278.

This change is

highfive · 2016-04-01T00:48:06Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/script_thread.rs

highfive · 2016-04-01T00:48:07Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify script code, but no tests are modified. Please consider adding a test!

paulrouget · 2016-04-01T03:29:18Z

We should start documenting these preferences somewhere.

mbrubeck · 2016-04-01T04:18:51Z

Could you add these to resources/prefs.json with the default value true? This will provide one easy way to discover and flip these prefs. Thanks!

Also, ./mach test-tidy failed:

./components/script/script_thread.rs:61: use statement is not in alphabetical order
    expected: js::jsapi::{JSObject, SetPreserveWrapperCallback}
    found: js::jsapi::RuntimeOptionsRef

bors-servo · 2016-04-02T01:44:25Z

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

mbrubeck · 2016-04-04T17:55:30Z

components/script/script_thread.rs

+        unsafe {
+            let rt_opts = RuntimeOptionsRef(runtime.rt());
+            is_baseline_enabled.map(|val| (*rt_opts).set_baseline_(val));
+            is_ion_enabled.map(|val| (*rt_opts).set_ion_(val));


I missed this before: Could you change this to use if let instead of .map()? This is purely a style preference; we find it more readable than map in cases where the return value isn't used.

r=mbrubeck with that change

When creating a runtime (script.rs), we check prefs and then enable or disable the JITs (Baseline and Ion).

mbrubeck · 2016-04-04T20:46:01Z

@bors-servo r+

mbrubeck · 2016-04-04T20:46:01Z

@bors-servo r+

bors-servo · 2016-04-04T20:46:01Z

📌 Commit 3ad7119 has been approved by mbrubeck

bors-servo · 2016-04-04T20:46:01Z

📌 Commit 3ad7119 has been approved by mbrubeck

mbrubeck · 2016-04-04T20:46:26Z

@bors-servo r+

bors-servo · 2016-04-04T20:46:27Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Inlined content_changed #10391

bors-servo · 2016-04-04T20:46:27Z

📌 Commit 3ad7119 has been approved by mbrubeck

mbrubeck · 2016-04-04T20:46:40Z

@bors-servo r+

bors-servo · 2016-04-04T20:46:41Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Inlined content_changed #10391

bors-servo · 2016-04-04T20:46:42Z

📌 Commit 3ad7119 has been approved by mbrubeck

bors-servo · 2016-04-04T21:14:10Z

⌛ Testing commit 3ad7119 with merge 60e5755...

Add preferences to enable/disable the JITs When creating a runtime (script.rs), we check prefs and then enable or disable the JITs (Baseline and Ion) accordingly. Closes #10278.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10325)

bors-servo · 2016-04-04T21:57:13Z

💔 Test failed - linux-rel

mbrubeck · 2016-04-04T22:36:55Z

@bors-servo retry

Intermittent timeout in /cssom-view/scrollingElement.html #10398

bors-servo · 2016-04-04T22:40:28Z

⚡ Previous build results for android, arm32, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor are reusable. Rebuilding only linux-rel...

bors-servo · 2016-04-04T23:22:23Z

💔 Test failed - linux-rel

mbrubeck · 2016-04-05T00:06:26Z

@bors-servo retry

Intermittent timeout in /cssom-view/scrollingElement.html #10398

bors-servo · 2016-04-05T00:20:14Z

⌛ Testing commit 3ad7119 with merge 863c905...

Add preferences to enable/disable the JITs When creating a runtime (script.rs), we check prefs and then enable or disable the JITs (Baseline and Ion) accordingly. Closes #10278.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10325)

bors-servo · 2016-04-05T01:17:37Z

☀️ Test successful - android, arm32, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor

highfive assigned mbrubeck Apr 1, 2016

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 1, 2016

mbrubeck added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-fails-tidy `./mach test-tidy` reported errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 1, 2016

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Apr 1, 2016

highfive added the S-needs-rebase There are merge conflict errors. label Apr 2, 2016

jdm removed S-fails-tidy `./mach test-tidy` reported errors. S-needs-rebase There are merge conflict errors. labels Apr 2, 2016

mbrubeck reviewed Apr 4, 2016
View reviewed changes

jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 4, 2016

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Apr 4, 2016

Add preferences to enable/disable the JITs

3ad7119

When creating a runtime (script.rs), we check prefs and then enable or disable the JITs (Baseline and Ion).

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 Apr 4, 2016

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 4, 2016

highfive added the S-tests-failed The changes caused existing tests to fail. label Apr 4, 2016

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 5, 2016

bors-servo merged commit 3ad7119 into servo:master Apr 5, 2016

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 5, 2016

bors-servo mentioned this pull request Apr 5, 2016

Refactors some entities from script_thread into script_runtime #10342

Merged

Uh oh!

Add preferences to enable/disable the JITs #10325

Add preferences to enable/disable the JITs #10325

Uh oh!

Conversation

ghost commented Apr 1, 2016

Uh oh!

highfive commented Apr 1, 2016

Uh oh!

highfive commented Apr 1, 2016

Uh oh!

paulrouget commented Apr 1, 2016

Uh oh!

mbrubeck commented Apr 1, 2016

Uh oh!

bors-servo commented Apr 2, 2016

Uh oh!

mbrubeck Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

mbrubeck commented Apr 4, 2016

Uh oh!

mbrubeck commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

mbrubeck commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

mbrubeck commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

mbrubeck commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

bors-servo commented Apr 4, 2016

Uh oh!

mbrubeck commented Apr 5, 2016

Uh oh!

bors-servo commented Apr 5, 2016

Uh oh!

bors-servo commented Apr 5, 2016

Uh oh!

Uh oh!