servo: Merge #17198 - Increase the size of the style sharing cache to 31 (from bzbarsky:bigger-sharing-cache); r=bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 07 Jun 2017 04:13:50 -0700
changeset 410930 b84549aaa1037161156416e1883295da2311ed3a
parent 410929 8e6503872bec65706914a92ddf776c2c72a7df7a
child 410931 be624d4f3f132c032429381517a2e736563dac38
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs17198, 1369621
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #17198 - Increase the size of the style sharing cache to 31 (from bzbarsky:bigger-sharing-cache); r=bholley Still a lot of guesswork here, but this does seem to get us better sharing. See https://bugzilla.mozilla.org/show_bug.cgi?id=1369621 for some data. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix https://bugzilla.mozilla.org/show_bug.cgi?id=1369621 <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: fdd1d719f34dccb2e808f91977bf134dc0bc6ab7
servo/components/style/parallel.rs
servo/components/style/sharing/mod.rs
--- a/servo/components/style/parallel.rs
+++ b/servo/components/style/parallel.rs
@@ -32,28 +32,30 @@ use std::borrow::Borrow;
 use std::mem;
 use time;
 use traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken};
 
 /// The maximum number of child nodes that we will process as a single unit.
 ///
 /// Larger values will increase style sharing cache hits and general DOM locality
 /// at the expense of decreased opportunities for parallelism. The style sharing
-/// cache can hold 8 entries, but not all styles are shareable, so we set this
-/// value to 16. These values have not been measured and could potentially be
-/// tuned.
+/// cache can hold 31 entries, but not all styles are shareable, so we set this
+/// value to 16. The size of the cache has been measured to provide pretty good
+/// sharing on a few pages, but could probably use more measurement and tuning.
+/// The work unit size is a bit of a guess at the moment; again could use
+/// measurement and tuning.
 pub const WORK_UNIT_MAX: usize = 16;
 
 /// Verify that the style sharing cache size doesn't change. If it does, we should
 /// reconsider the above. We do this, rather than defining WORK_UNIT_MAX in terms
 /// of STYLE_SHARING_CANDIDATE_CACHE_SIZE, so that altering the latter doesn't
 /// have surprising effects on the parallelism characteristics of the style system.
 #[allow(dead_code)]
 fn static_assert() {
-    unsafe { mem::transmute::<_, [u32; STYLE_SHARING_CANDIDATE_CACHE_SIZE]>([1; 8]); }
+    unsafe { mem::transmute::<_, [u32; STYLE_SHARING_CANDIDATE_CACHE_SIZE]>([1; 31]); }
 }
 
 /// A list of node pointers.
 ///
 /// Note that the inline storage doesn't need to be sized to WORK_UNIT_MAX, but
 /// it generally seems sensible to do so.
 type NodeList<N> = SmallVec<[SendNode<N>; WORK_UNIT_MAX]>;
 
--- a/servo/components/style/sharing/mod.rs
+++ b/servo/components/style/sharing/mod.rs
@@ -79,18 +79,20 @@ use selectors::matching::{ElementSelecto
 use smallvec::SmallVec;
 use std::mem;
 use std::ops::Deref;
 use stylist::{ApplicableDeclarationBlock, Stylist};
 
 mod checks;
 
 /// The amount of nodes that the style sharing candidate cache should hold at
-/// most.
-pub const STYLE_SHARING_CANDIDATE_CACHE_SIZE: usize = 8;
+/// most.  We'd somewhat like 32, but ArrayDeque only implements certain backing
+/// store sizes.  A cache size of 32 would mean a backing store of 33, but
+/// that's not an implemented size: we can do 32 or 40.
+pub const STYLE_SHARING_CANDIDATE_CACHE_SIZE: usize = 31;
 
 /// Controls whether the style sharing cache is used.
 #[derive(Clone, Copy, PartialEq)]
 pub enum StyleSharingBehavior {
     /// Style sharing allowed.
     Allow,
     /// Style sharing disallowed.
     Disallow,