stylo: Drop thread pool stack size to 100k

NEW
Assigned to

Status

()

Core
CSS Parsing and Computation
P2
normal
26 days ago
12 days ago

People

(Reporter: bholley, Assigned: jseward)

Tracking

(Depends on: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P1])

Once we get rid of dom-level recursion during the parallel traversal in the dependencies, we should be able to get by with very small stacks for the stylo workers. I was originally thinking 500k, but I think even less would be sufficient.

We should be just call:

stack_size(self, 100 * 1024)
Depends on: 1376884
Whiteboard: [MemShrink]

Comment 1

26 days ago
What is the benefit of a small stack size if we don't actually fault the memory in?  Seems like some increased stability risk.
(Assignee)

Comment 2

26 days ago
(In reply to Ben Kelly [reviewing, but slowly][:bkelly] from comment #1)
> What is the benefit of a small stack size if we don't actually fault the
> memory in?  Seems like some increased stability risk.

I agree; this strikes me as dangerous, for (to me) unclear benefit.
Typically we'd only have 6 stylo workers with minimal physical memory use in
each stack.  I would be reluctant to drop the stack size below 1MB.

At 100KB we would be able only to get in about 60 frames in the recursive
parallel traversal, per my measurements from last week, before we'd have to
disable the tail recursion optimisation.  And that's assuming there are no
other frames on the stack.
(Assignee)

Comment 3

26 days ago
(In reply to Julian Seward [:jseward] from comment #2)
> At 100KB we would be able only to get in about 60 frames in the recursive
> parallel traversal, per my measurements from last week,

Not even 60 frames.  For an x86_64-linux optimised Gecko build, we use 3472
bytes of stack per recursive invokation.  So a 100KB stack would limit us to,
realistically, about 20 iterations.
(In reply to Ben Kelly [reviewing, but slowly][:bkelly] from comment #1)
> What is the benefit of a small stack size if we don't actually fault the
> memory in?  Seems like some increased stability risk.

Modulo the dependencies, we don't have any required recursion in the parallel paths of the style system. So the amount of recursion we need is under our control, and the amount of headroom we need is fixed.

The benefits are:
* Saving address space on 32-bit.
* Avoiding dirtying the entire stack with a single long tail call, which would commit the memory for the lifetime of the process.

I think we should:
(1) Fix emilio's invalidation stuff to user iterators (see the dependent bug).
(2) Set the tail call limit to something conservative (~20 is totally fine). Figure out how much stack space this uses.
(3) Measure the maximum stack space used to style an element, and multiply it a few times to be safe.
(4) Set the stack size limit to (2) + (3). This happens at http://searchfox.org/mozilla-central/rev/17ebac68112bd635b458e831670c1e506ebc17ad/servo/components/style/gecko/global_style_data.rs#84


Julian is going to look at this.
Assignee: nobody → jseward
Priority: -- → P2
Whiteboard: [MemShrink] → [MemShrink:P1]
You need to log in before you can comment on or make changes to this bug.