Minor cleanup and optimizations in glyph/style caching #11180

mbrubeck · 2016-05-14T01:18:04Z

Gets rid of some unnecessary String and Arc clones during text shaping and style matching.

This change is


        Remove unnecessary pub


        Remove unnecessary ServoFont type alias


        Make some Font fields private

highfive · 2016-05-14T01:18:08Z

Heads up! This PR modifies the following files:

@bholley: components/style/matching.rs


        Eliminate unneeded clones in find_or_create

...and use it to eliminate duplicate hash lookups and string copies in shape_text.


        Remove unnecessary clone in LRUCache::touch

highfive · 2016-05-14T01:21:08Z

New code was committed to pull request.

pcwalton · 2016-05-14T16:06:51Z

@bors-servo: r+

bors-servo · 2016-05-14T16:06:52Z

📌 Commit dc6be7b has been approved by pcwalton

bors-servo · 2016-05-14T16:06:55Z

⌛ Testing commit dc6be7b with merge e299076...


        Auto merge of #11180 - mbrubeck:text-cleanup, r=pcwalton

@pcwalton

Minor cleanup and optimizations in glyph/style caching Gets rid of some unnecessary String and Arc clones during text shaping and style matching. r? @pcwalton  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11180)

bors-servo · 2016-05-14T16:43:14Z

💔 Test failed - mac-rel-wpt

highfive · 2016-05-14T16:43:17Z

  ▶ 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;, ../src/libstd/rand/mod.rs:159
  │    1:        0x10af5aa48 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │ 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
  │    2:        0x10af60e65 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x10af60a7e - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x10a6cba92 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │ 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
  │    5:        0x10af48118 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │ 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
  │    6:        0x10af49a4e - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:        0x10af61489 - std::rand::thread_rng::THREAD_RNG_KEY::__init::h536e3917997a1628
  │    8:        0x10af4871c - std::rand::thread_rng::h789d00d1fce4f79b
  │    9:        0x109a1493c - _&lt;collections..hash..map..RandomState as core..default..Default&gt;::default::ha847494db5044892
  │   10:        0x109a52825 - net::http_loader::load::h981055a57a1b11f1
  │   11:        0x109a4eff0 - std::sys_common::unwind::try::try_fn::h545b21ec982a5e96
  │   12:        0x10af59cab - __rust_try
  │   13:        0x10af59c33 - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   14:        0x109a50cf1 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   15:        0x10af5fee8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   16:     0x7fff86bcf059 - _pthread_body
  │   17:     0x7fff86bcefd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x10af5aa48 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │ 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
  │    2:        0x10af60e65 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │ 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
  │    3:        0x10af60a7e - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x10a6cba92 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x10af48118 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │    6:        0x10af49a4e - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:        0x10af61489 - std::rand::thread_rng::THREAD_RNG_KEY::__init::h536e3917997a1628
  │    8:        0x10af4871c - std::rand::thread_rng::h789d00d1fce4f79b
  │    9:        0x109a1493c - _&lt;collections..hash..map..RandomState as core..default..Default&gt;::default::ha847494db5044892
  │   10:        0x109a52825 - net::http_loader::load::h981055a57a1b11f1
  │   11:        0x109a4eff0 - std::sys_common::unwind::try::try_fn::h545b21ec982a5e96
  │   12:        0x10af59cab - __rust_try
  │   13:        0x10af59c33 - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   14:        0x109a50cf1 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   15:        0x10af5fee8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   16:     0x7fff86bcf059 - _pthread_body
  │   17:     0x7fff86bcefd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x10af5aa48 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x10af60e65 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x10af60a7e - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x10a6cba92 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x10af48118 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │    6:        0x10af49a4e - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:        0x10ae9e6ff - rand::thread_rng::THREAD_RNG_KEY::__init::h4e1387a2e4a21393
  │    8:        0x10ae9e53c - rand::thread_rng::he394b446ab605489
  │    9:        0x10ae67bd2 - uuid::Uuid::new_v4::hc206acac9c7e580d
  │   10:        0x109a534eb - net::http_loader::load::h981055a57a1b11f1
  │   11:        0x109a4eff0 - std::sys_common::unwind::try::try_fn::h545b21ec982a5e96
  │   12:        0x10af59cab - __rust_try
  │   13:        0x10af59c33 - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   14:        0x109a50cf1 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   15:        0x10af5fee8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   16:     0x7fff86bcf059 - _pthread_body
  │   17:     0x7fff86bcefd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x10af5aa48 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x10af60e65 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x10af60a7e - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x10a6cba92 - util::panicking::initiate_panic_hook::_$u7b$$u</span><span class="stdout">7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x10af48118 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │    6:        0x10af49a4e - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:        0x10ae9e6ff - rand::thread_rng::THREAD_RNG_KEY::__init::h4e1387a2e4a21393
  │    8:        0x10ae9e53c - rand::thread_rng::he394b446ab605489
  │    9:        0x10ae67bd2 - uuid::Uuid::new_v4::hc206acac9c7e580d
  │   10:        0x109a534eb - net::http_loader::load::h981055a57a1b11f1
  │   11:        0x109a4eff0 - std::sys_common::unwind::try::try_fn::h545b21ec982a5e96
  │   12:        0x10af59cab - __rust_try
  │   13:        0x10af59c33 - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   14:        0x109a50cf1 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   15:        0x10af5fee8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   16:     0x7fff86bcf059 - _pthread_body
  │   17:     0x7fff86bcefd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x10af5aa48 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x10af60e65 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x10af60a7e - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x10a6cba92 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x10af48118 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │    6:        0x10af49a4e - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:        0x10ae9e6ff - rand::thread_rng::THREAD_RNG_KEY::__init::h4e1387a2e4a21393
  │    8:        0x10ae9e53c - rand::thread_rng::he394b446ab605489
  │    9:        0x10ae67bd2 - uuid::Uuid::new_v4::hc206acac9c7e580d
  │   10:        0x109a534eb - net::http_loader::load::h981055a57a1b11f1
  │   11:        0x109a4eff0 - std::sys_common::unwind::try::try_fn::h545b21ec982a5e96
  │   12:        0x10af59cab - __rust_try
  │   13:        0x10af59c33 - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   14:        0x109a50cf1 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   15:        0x10af5fee8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   16:     0x7fff86bcf059 - _pthread_body
  │   17:     0x7fff86bcefd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x10af5aa48 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x10af60e65 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x10af60a7e - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x10a6cba92 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x10af48118 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │    6:        0x10af49a4e - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:        0x10af61489 - std::rand::thread_rng::THREAD_RNG_KEY::__init::h536e3917997a1628
  │    8:        0x10af4871c - std::rand::thread_rng::h789d00d1fce4f79b
  │    9:        0x109a1493c - _&lt;collections..hash..map..RandomState as core..default..Default&gt;::default::ha847494db5044892
  │   10:        0x109a52825 - net::http_loader::load::h981055a57a1b11f1
  │   11:        0x109a4eff0 - std::sys_common::unwind::try::try_fn::h545b21ec982a5e96
  │   12:        0x10af59cab - __rust_try
  │   13:        0x10af59c33 - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   14:        0x109a50cf1 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   15:        0x10af5fee8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   16:     0x7fff86bcf059 - _pthread_body
  │   17:     0x7fff86bcefd6 - _pthread_start
  │ stack backtrace:
  │    1:        0x10af5aa48 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:        0x10af60e65 - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:        0x10af60a7e - std::panicking::default_hook::hc2c969e7453d080c
  │    4:        0x10a6cba92 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha2bd86c312dc8d7a
  │    5:        0x10af48118 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │    6:        0x10af49a4e - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:        0x10af61489 - std::rand::thread_rng::THREAD_RNG_KEY::__init::h536e3917997a1628
  │    8:        0x10af4871c - std::rand::thread_rng::h789d00d1fce4f79b
  │    9:        0x109a1493c - _&lt;collections..hash..map..RandomState as core..default..Default&gt;::default::ha847494db5044892
  │   10:        0x109a52825 - net::http_loader::load::h981055a57a1b11f1
  │   11:        0x109a4eff0 - std::sys_common::unwind::try::try_fn::h545b21ec982a5e96
  │   12:        0x10af59cab - __rust_try
  │   13:        0x10af59c33 - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   14:        0x109a50cf1 - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::hdb3e6de044792225
  │   15:        0x10af5fee8 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   16:     0x7fff86bcf059 - _pthread_body
  └   17:     0x7fff86bcefd6 - _pthread_start

jdm · 2016-05-14T16:57:26Z

@bors-servo: retry

#11100

bors-servo · 2016-05-14T16:57:28Z

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

bors-servo · 2016-05-14T17:30:54Z

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

mbrubeck added 3 commits May 13, 2016

Remove unnecessary pub

45dc587

Remove unnecessary ServoFont type alias

4361f92

Make some Font fields private

1c5ec6f

highfive assigned pcwalton May 14, 2016

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

mbrubeck added 2 commits May 14, 2016

Eliminate unneeded clones in find_or_create

d2717c4

...and use it to eliminate duplicate hash lookups and string copies in shape_text.

Remove unnecessary clone in LRUCache::touch

Loading status checks…

dc6be7b

mbrubeck force-pushed the mbrubeck:text-cleanup branch from fafbb94 to dc6be7b May 14, 2016

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

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

bors-servo merged commit dc6be7b into servo:master May 14, 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 14, 2016

mbrubeck deleted the mbrubeck:text-cleanup branch May 17, 2016

servo / servo

Minor cleanup and optimizations in glyph/style caching #11180

Minor cleanup and optimizations in glyph/style caching #11180

mbrubeck commented May 14, 2016 •

edited by larsbergstrom

highfive commented May 14, 2016

highfive commented May 14, 2016

pcwalton commented May 14, 2016

bors-servo commented May 14, 2016

bors-servo commented May 14, 2016

bors-servo commented May 14, 2016

highfive commented May 14, 2016

jdm commented May 14, 2016

bors-servo commented May 14, 2016

bors-servo commented May 14, 2016

servo / servo

Join GitHub today

Minor cleanup and optimizations in glyph/style caching #11180

Minor cleanup and optimizations in glyph/style caching #11180

Conversation

mbrubeck commented May 14, 2016 • edited by larsbergstrom

highfive commented May 14, 2016

highfive commented May 14, 2016

pcwalton commented May 14, 2016

bors-servo commented May 14, 2016

bors-servo commented May 14, 2016

bors-servo commented May 14, 2016

highfive commented May 14, 2016

jdm commented May 14, 2016

bors-servo commented May 14, 2016

bors-servo commented May 14, 2016

mbrubeck commented May 14, 2016 •

edited by larsbergstrom