servo: Merge #18412 - Increase stack safety margin for stylo (from bholley:bump_style_stacks); r=bholley
authorBobby Holley <bobbyholley@gmail.com>
Thu, 07 Sep 2017 13:52:48 -0500
changeset 379532 95543c372e9f9def5a42d71d4d5356917a031a89
parent 379531 bf29e38baf5d25933128d6e564ae732a422a2837
child 379533 3a39f8baf4ecfb635cb92da56054970985f8e98b
push id50693
push userservo-vcs-sync@mozilla.com
push dateThu, 07 Sep 2017 19:54:04 +0000
treeherderautoland@95543c372e9f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
milestone57.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 #18412 - Increase stack safety margin for stylo (from bholley:bump_style_stacks); r=bholley https://bugzilla.mozilla.org/show_bug.cgi?id=1395708 Source-Repo: https://github.com/servo/servo Source-Revision: 8868d2223dbb28b07ae1936095bc9ec644fe58a7
servo/components/style/parallel.rs
--- a/servo/components/style/parallel.rs
+++ b/servo/components/style/parallel.rs
@@ -27,33 +27,36 @@ use context::{StyleContext, ThreadLocalS
 use dom::{OpaqueNode, SendNode, TElement};
 use itertools::Itertools;
 use rayon;
 use scoped_tls::ScopedTLS;
 use smallvec::SmallVec;
 use traversal::{DomTraversal, PerLevelTraversalData};
 
 /// The minimum stack size for a thread in the styling pool, in kilobytes.
-pub const STYLE_THREAD_STACK_SIZE_KB: usize = 128;
+pub const STYLE_THREAD_STACK_SIZE_KB: usize = 256;
 
 /// The stack margin. If we get this deep in the stack, we will skip recursive
 /// optimizations to ensure that there is sufficient room for non-recursive work.
 ///
-/// When measured with 128KB stacks and 40KB margin, we could support 53
-/// levels of recursion before the limiter kicks in, on x86_64-Linux [1].
+/// We allocate large safety margins because certain OS calls can use very large
+/// amounts of stack space [1]. Reserving a larger-than-necessary stack costs us
+/// address space, but if we keep our safety margin big, we will generally avoid
+/// committing those extra pages, and only use them in edge cases that would
+/// otherwise cause crashes.
 ///
-/// We currently use 45KB margins, because that seems to be the minimum amount
-/// of head room required by an unoptimized debug build, as measured on [2]. We
-/// could probably get away with a smaller margin on optimized release builds,
-/// but that would be a pain, and the extra padding reduces stability risk.
+/// When measured with 128KB stacks and 40KB margin, we could support 53
+/// levels of recursion before the limiter kicks in, on x86_64-Linux [2]. When
+/// we doubled the stack size, we added it all to the safety margin, so we should
+/// be able to get the same amount of recursion.
 ///
-/// [1] See Gecko bug 1376883 for more discussion on the measurements.
-/// [2] layout/style/crashtests/1383981-3.html
+/// [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1395708#c15
+/// [2] See Gecko bug 1376883 for more discussion on the measurements.
 ///
-pub const STACK_SAFETY_MARGIN_KB: usize = 45;
+pub const STACK_SAFETY_MARGIN_KB: usize = 168;
 
 /// 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.  There
 /// are some measurements in
 /// https://bugzilla.mozilla.org/show_bug.cgi?id=1385982#c11 and comments 12
 /// and 13 that investigate some slightly different values for the work unit