1375911 - stylo: avoid spinning up the style thread pool in the parent process

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P2)

Description

[Bobby Holley (:bholley)]

In bug 1374824 comment 9, glandium determined that avoiding firing up the rayon thread pool saves significant memory, since we have per-thread jemalloc arenas.

Given that we don't immediately plan to enable stylo for chrome documents, we should avoid creating this thread pool in the parent process. There might be some incidental stylo-driven styling of html documents going on in the parent process (which would currently trigger the thread-pool creation), but that should be fine to run in sequential mode.

Comment 1

[Bobby Holley (:bholley)]

Oh, and the patch itself here should be trivial - we just want to do an FFI call during threadpool creation (around where we check the STYLO_THREADS environmental variable), and force to None if we're in the parent process. Probably worth building in an env-var opt-out while we're at it.

Comment 2

[Cameron McCormack (:heycam)]

We should make sure not to do this when e10s is disabled, so that our current e10s-disabled tests continue running in parallel mode.

Comment 3

[Bobby Holley (:bholley)]

Attachment: Avoid creating a stylo thread pool in e10s parent processes. v1

MozReview-Commit-ID: LW92yNDKZf4

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74df08122c3d
Avoid creating a stylo thread pool in e10s parent processes. r=xidorn

Comment 5

[Bobby Holley (:bholley)]

Servo-side changes: https://github.com/servo/servo/pull/18137

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/74df08122c3d

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Reading this code again, I actually get confused that, why do we need to do this at all?

STYLE_THREAD_POOL is a lazy static, and it is only dereferenced when traverse_subtree is called. It means that if no document is using stylo, then style threads would never be created. So why do we bother adding another gate at all?

Comment 8

[Emilio Cobos Álvarez (:emilio)]

(In reply to Xidorn Quan [:xidorn] UTC+10 (less responsive Nov 5 ~ Dec 16) from comment #7)
> Reading this code again, I actually get confused that, why do we need to do
> this at all?
> 
> STYLE_THREAD_POOL is a lazy static, and it is only dereferenced when
> traverse_subtree is called. It means that if no document is using stylo,
> then style threads would never be created. So why do we bother adding
> another gate at all?

It was done because there were about:blank tabs and what not that were calling into style, IIRC.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #8)
> It was done because there were about:blank tabs and what not that were
> calling into style, IIRC.

OK, that makes sense, then. I guess we can still go ahead removing it anyway because we now have another gate added in bug 1398993 to disable stylo in e10s parent process.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Yup, that makes sense to me.