script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. #11205

pcwalton · 2016-05-16T18:46:22Z

This skips useless message traffic when requestAnimationFrame() is
called during an animation frame callback. It reduces CPU usage of the
following snippet by 49%:

<script>
    function foo() {
        requestAnimationFrame(foo);
    }
</script>
<button onclick="foo()">Start rAF</button>

Partially addresses #9844.

This change is

highfive · 2016-05-16T18:46:25Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/dom/document.rs

highfive · 2016-05-16T18:46:26Z

Warning

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

pcwalton · 2016-05-16T18:46:33Z

r? @jdm

jdm · 2016-05-16T21:03:15Z

components/script/dom/document.rs

@@ -201,6 +201,7 @@ pub struct Document {
    /// List of animation frame callbacks
    #[ignore_heap_size_of = "closures are hard"]
    animation_frame_list: DOMRefCell<BTreeMap<u32, Box<FnBox(f64)>>>,
+    running_animation_callbacks: Cell<bool>,


Document this, please.

jdm · 2016-05-16T21:03:25Z

r=me with documentation added.

pcwalton · 2016-05-17T17:22:47Z

@bors-servo: r=jdm

bors-servo · 2016-05-17T17:22:48Z

📌 Commit 02ab54a has been approved by jdm

pcwalton · 2016-05-17T17:23:36Z

@bors-servo: r-

highfive · 2016-05-17T17:23:40Z

New code was committed to pull request.

pcwalton · 2016-05-17T17:23:42Z

@bors-servo: r=jdm

bors-servo · 2016-05-17T17:23:43Z

📌 Commit 8a83261 has been approved by jdm


        script: Avoid needless `ChangeRunningAnimationsState` messages during

typical `requestAnimationFrame()` animations. This skips useless message traffic when `requestAnimationFrame()` is called during an animation frame callback. It reduces CPU usage of the following snippet by 49%: <script> function foo() { requestAnimationFrame(foo); } </script> <button onclick="foo()">Start rAF</button> Partially addresses #9844.

bors-servo · 2016-05-17T19:02:59Z

⌛ Testing commit 8a83261 with merge e123605...


        Auto merge of #11205 - pcwalton:avoid-needless-animation-state-change…

…s, r=jdm script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. This skips useless message traffic when `requestAnimationFrame()` is called during an animation frame callback. It reduces CPU usage of the following snippet by 49%: <script> function foo() { requestAnimationFrame(foo); } </script> <button onclick="foo()">Start rAF</button> Partially addresses #9844.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11205)

bors-servo · 2016-05-17T19:35:44Z

💔 Test failed - mac-rel-wpt

highfive · 2016-05-17T19:35:46Z

  ▶ TIMEOUT [expected OK] /html/dom/reflection-embedded.html
  │ 
  │ thread &#39;http_loader for http://web-platform.test:8000/html/dom/test-valueOf&#39; panicked at &#39;could not initialize thread_rng: Too many open files (os error 24)&#39;, ../src/libstd/rand/mod.rs:159
  │ stack backtrace:
  │ thread &#39;http_loader for http://web-platform.test:8000/html/dom/test-valueOf&#39; panicked at &#39;could not initialize thread_rng: Too many open files (os error 24)&#39;, /Users/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/rand-0.3.14/src/lib.rs:873
  │ thread &#39;http_loader for http://web-platform.test:8000/html/dom/test-valueOf&#39; panicked at &#39;could not initialize thread_rng: Too many open files (os error 24)&#39;, ../src/libstd/rand/mod.rs:159
  │    1:        0x109003728 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x109009bb5 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x1090097ce - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x1087457d2 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x108ff1442 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │    6:        0x10900a176 - std::panicking::begin_panic::h4889569716505182
  │    7:        0x108ff2d38 - std::panicking::begin_panic_fmt::h484cd47786497f03
  │    8:        0x10900a5e9 - std::rand::thread_rng::THREAD_RNG_KEY::__init::h536e3917997a1628
  │    9:        0x108ff1a4c - std::rand::thread_rng::h789d00d1fce4f79b
  │ thread &#39;http_loader for http://web-platform.test:8000/html/dom/test-valueOf&#39; panicked at &#39;could not initialize thread_rng: Too many open files (os error 24)&#39;, /Users/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/rand-0.3.14/src/lib.rs:873
  │ thread &#39;http_loader for http://web-platform.test:8000/html/dom/test-valueOf&#39; panicked at &#39;could not initialize thread_rng: Too many open files (os error 24)&#39;, /Users/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/rand-0.3.14/src/lib.rs:873
  │   10:        0x1078a524c - _&lt;collections..hash..map..RandomState as core..default..Default&gt;::default::ha847494db5044892
  │   11:        0x1078e32a0 - net::http_loader::load::h981055a57a1b11f1
  │   12:        0x1078dfa0b - std::panicking::try::call::h36f7dc6ba106d223
  │   13:        0x10900d33b - __rust_try
  │   14:        0x10900d2d5 - __rust_maybe_catch_panic
  │   15:        0x1078e1784 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   16:        0x109008bd8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   17:     0x7fff8c3c2059 - _pthread_body
  │   18:     0x7fff8c3c1fd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x109003728 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x109009bb5 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x1090097ce - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x1087457d2 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x108ff1442 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │    6:        0x10900a176 - std::panicking::begin_panic::h4889569716505182
  │    7:        0x108ff2d38 - std::panicking::begin_panic_fmt::h484cd47786497f03
  │    8:        0x10882d8cf - rand::thread_rng::THREAD_RNG_KEY::__init::h4e1387a2e4a21393
  │    9:        0x10882d70c - rand::thread_rng::he394b446ab605489
  │   10:        0x10876eb52 - uuid::Uuid::new_v4::hc206acac9c7e580d
  │   11:        0x1078e3f6b - net::http_loader::load::h981055a57a1b11f1
  │   12:        0x1078dfa0b - std::panicking::try::call::h36f7dc6ba106d223
  │   13:        0x10900d33b - __rust_try
  │   14:        0x10900d2d5 - __rust_maybe_catch_panic
  │   15:        0x1078e1784 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   16:        0x109008bd8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   17:     0x7fff8c3c2059 - _pthread_body
  │   18:     0x7fff8c3c1fd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x109003728 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x109009bb5 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x1090097ce - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x1087457d2 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x108ff1442 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │    6:        0x10900a176 - std::panicking::begin_panic::h4889569716505182
  │    7:        0x108ff2d38 - std::panicking::begin_panic_fmt::h484cd47786497f03
  │    8:        0x10900a5e9 - std::rand::thread_rng::THREAD_RNG_KEY::__init::h536e3917997a1628
  │    9:        0x108ff1a4c - std::rand::thread_rng::h789d00d1fce4f79b
  │   10:        0x1078a524c - _&lt;collections..hash..map..RandomState as core..default..Default&gt;::default::ha847494db5044892
  │   11:        0x1078e32a0 - net::http_loader::load::h981055a57a1b11f1
  │   12:        0x1078dfa0b - std::panicking::try::call::h36f7dc6ba106d223
  │   13:        0x10900d33b - __rust_try
  │   14:        0x10900d2d5 - __rust_maybe_catch_panic
  │   15:        0x1078e1784 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   16:        0x109008bd8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   17:     0x7fff8c3c2059 - _pthread_body
  │   18:     0x7fff8c3c1fd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x109003728 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x109009bb5 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x1090097ce - std::panicking::default_hook::hc2c969e7453d080c
  │    4:    </span><span class="stdout">    0x1087457d2 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x108ff1442 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │    6:        0x10900a176 - std::panicking::begin_panic::h4889569716505182
  │    7:        0x108ff2d38 - std::panicking::begin_panic_fmt::h484cd47786497f03
  │    8:        0x10882d8cf - rand::thread_rng::THREAD_RNG_KEY::__init::h4e1387a2e4a21393
  │    9:        0x10882d70c - rand::thread_rng::he394b446ab605489
  │   10:        0x10876eb52 - uuid::Uuid::new_v4::hc206acac9c7e580d
  │   11:        0x1078e3f6b - net::http_loader::load::h981055a57a1b11f1
  │   12:        0x1078dfa0b - std::panicking::try::call::h36f7dc6ba106d223
  │   13:        0x10900d33b - __rust_try
  │   14:        0x10900d2d5 - __rust_maybe_catch_panic
  │   15:        0x1078e1784 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   16:        0x109008bd8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   17:     0x7fff8c3c2059 - _pthread_body
  │   18:     0x7fff8c3c1fd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x109003728 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x109009bb5 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x1090097ce - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x1087457d2 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x108ff1442 - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │    6:        0x10900a176 - std::panicking::begin_panic::h4889569716505182
  │    7:        0x108ff2d38 - std::panicking::begin_panic_fmt::h484cd47786497f03
  │    8:        0x10882d8cf - rand::thread_rng::THREAD_RNG_KEY::__init::h4e1387a2e4a21393
  │    9:        0x10882d70c - rand::thread_rng::he394b446ab605489
  │   10:        0x10876eb52 - uuid::Uuid::new_v4::hc206acac9c7e580d
  │   11:        0x1078e3f6b - net::http_loader::load::h981055a57a1b11f1
  │   12:        0x1078dfa0b - std::panicking::try::call::h36f7dc6ba106d223
  │   13:        0x10900d33b - __rust_try
  │   14:        0x10900d2d5 - __rust_maybe_catch_panic
  │   15:        0x1078e1784 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   16:        0x109008bd8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   17:     0x7fff8c3c2059 - _pthread_body
  └   18:     0x7fff8c3c1fd6 - _pthread_start

jdm · 2016-05-17T20:10:27Z

@bors-servo: retry

#11100


        Auto merge of #11205 - pcwalton:avoid-needless-animation-state-change…

…s, r=jdm script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. This skips useless message traffic when `requestAnimationFrame()` is called during an animation frame callback. It reduces CPU usage of the following snippet by 49%: <script> function foo() { requestAnimationFrame(foo); } </script> <button onclick="foo()">Start rAF</button> Partially addresses #9844.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11205)

bors-servo · 2016-05-17T21:05:02Z

⌛ Testing commit 8a83261 with merge 6f0ba7d...

bors-servo · 2016-05-17T21:57:31Z

💔 Test failed - linux-rel

highfive · 2016-05-17T21:57:34Z

  ▶ TIMEOUT [expected FAIL] /css21_dev/html4/fixed-table-layout-003c07.htm
  │ 
  │ Xlib:  extension &#34;XFree86-VidModeExtension&#34; missing on display &#34;:0&#34;.
  │ Shutting down the Constellation after generating an output file or exit flag specified
  │ Xlib:  extension &#34;XFree86-VidModeExtension&#34; missing on display &#34;:0&#34;.
  │ Shutting down the Constellation after generating an output file or exit flag specified
  │ thread &#39;Constellation&#39; panicked at &#39;Couldn&#39;t receive FontCacheThread reply: IoError(Error { repr: Os { code: 104, message: &#34;Connection reset by peer&#34; } })&#39;, ../src/libcore/result.rs:785
  │ stack backtrace:
  │    1:     0x7fb97f4d3a40 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:     0x7fb97f4db54b - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:     0x7fb97f4db1b3 - std::panicking::default_hook::hc2c969e7453d080c
  │    4:     0x7fb97ea37437 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::he2b22674ad1748f3
  │    5:     0x7fb97f4c150c - std::panicking::rust_panic_with_hook::hfe203e3083c2b544
  │    6:     0x7fb97f4db791 - std::panicking::begin_panic::h4889569716505182
  │    7:     0x7fb97f4c2f0a - std::panicking::begin_panic_fmt::h484cd47786497f03
  │    8:     0x7fb97f4db72e - rust_begin_unwind
  │    9:     0x7fb97f51288f - core::panicking::panic_fmt::h257ceb0aa351d801
  │   10:     0x7fb97e7b5b83 - core::result::unwrap_failed::h1d94443d858a91df
  │   11:     0x7fb97e7dc14a - gfx::font_cache_thread::FontCacheThread::exit::h335f05e55068200a
  │   12:     0x7fb97daab278 - _&lt;compositing..Constellation&lt;LTF, STF&gt;&gt;::handle_exit::hcddb9572417a6d0e
  │   13:     0x7fb97da80bfd - _&lt;compositing..Constellation&lt;LTF, STF&gt;&gt;::handle_request::h4c02e715c488d095
  │   14:     0x7fb97da7480c - std::panicking::try::call::h5cb361f6c9810866
  │   15:     0x7fb97f4e5e3b - __rust_try
  │   16:     0x7fb97f4e5dde - __rust_maybe_catch_panic
  │   17:     0x7fb97da75ea5 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::h477ff2eb06cc3d05
  │   18:     0x7fb97f4d99a4 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   19:     0x7fb97b48e181 - start_thread
  │   20:     0x7fb97afa547c - __clone
  └   21:                0x0 - &lt;unknown&gt;

jdm · 2016-05-17T22:02:29Z

@bors-servo: retry

#8815

bors-servo · 2016-05-18T00:24:47Z

⌛ Testing commit 8a83261 with merge e5c3bb8...


        Auto merge of #11205 - pcwalton:avoid-needless-animation-state-change…

…s, r=jdm script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. This skips useless message traffic when `requestAnimationFrame()` is called during an animation frame callback. It reduces CPU usage of the following snippet by 49%: <script> function foo() { requestAnimationFrame(foo); } </script> <button onclick="foo()">Start rAF</button> Partially addresses #9844.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11205)

bors-servo · 2016-05-18T01:15:13Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned wafflespeanut May 16, 2016

highfive added the S-awaiting-review label May 16, 2016

highfive assigned jdm and unassigned wafflespeanut May 16, 2016

jdm reviewed May 16, 2016
View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels May 16, 2016

highfive added S-awaiting-merge and removed S-needs-code-changes labels May 17, 2016

pcwalton force-pushed the pcwalton:avoid-needless-animation-state-changes branch from 02ab54a to 8a83261 May 17, 2016

highfive removed the S-awaiting-merge label May 17, 2016

highfive added the S-awaiting-review label May 17, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels May 17, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels May 17, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels May 17, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels May 17, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels May 18, 2016

bors-servo merged commit 8a83261 into servo:master May 18, 2016
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

highfive removed the S-awaiting-merge label May 18, 2016

nox mentioned this pull request May 18, 2016

High CPU usage during a RAF loop #9844

Closed

paulrouget mentioned this pull request May 19, 2016

requestAnimationFrame runs at 1 FPS #11260

Closed

servo / servo

script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. #11205

script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. #11205

pcwalton commented May 16, 2016 •

edited by larsbergstrom

highfive commented May 16, 2016

highfive commented May 16, 2016

pcwalton commented May 16, 2016

This comment has been minimized.

jdm commented May 16, 2016

pcwalton commented May 17, 2016

bors-servo commented May 17, 2016

pcwalton commented May 17, 2016

highfive commented May 17, 2016

pcwalton commented May 17, 2016

bors-servo commented May 17, 2016

bors-servo commented May 17, 2016

bors-servo commented May 17, 2016

highfive commented May 17, 2016

jdm commented May 17, 2016

bors-servo commented May 17, 2016

bors-servo commented May 17, 2016

highfive commented May 17, 2016

jdm commented May 17, 2016

bors-servo commented May 18, 2016

bors-servo commented May 18, 2016

servo / servo

Join GitHub today

script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. #11205

script: Avoid needless `ChangeRunningAnimationsState` messages during typical `requestAnimationFrame()` animations. #11205

Conversation

pcwalton commented May 16, 2016 • edited by larsbergstrom

highfive commented May 16, 2016

highfive commented May 16, 2016

pcwalton commented May 16, 2016

This comment has been minimized.

jdm May 16, 2016

jdm commented May 16, 2016

pcwalton commented May 17, 2016

bors-servo commented May 17, 2016

pcwalton commented May 17, 2016

highfive commented May 17, 2016

pcwalton commented May 17, 2016

bors-servo commented May 17, 2016

bors-servo commented May 17, 2016

bors-servo commented May 17, 2016

highfive commented May 17, 2016

jdm commented May 17, 2016

bors-servo commented May 17, 2016

bors-servo commented May 17, 2016

highfive commented May 17, 2016

jdm commented May 17, 2016

bors-servo commented May 18, 2016

bors-servo commented May 18, 2016

pcwalton commented May 16, 2016 •

edited by larsbergstrom