feat(blitz-dom): expose stylo thread count via DocumentConfig#436
Closed
HenriqueAnzoategui wants to merge 1 commit into
Closed
feat(blitz-dom): expose stylo thread count via DocumentConfig#436HenriqueAnzoategui wants to merge 1 commit into
HenriqueAnzoategui wants to merge 1 commit into
Conversation
Stylo's `STYLE_THREAD_POOL` is a process-global LazyLock-initialised rayon pool. Style traversal on each worker holds a thread-local `AtomicRefCell<SharingCache>` borrowed mutably for the traversal's lifetime, so if two concurrent traversals (from separate `Document`s on separate OS threads) land on the same rayon worker, the second panics with `already mutably borrowed`. Embedders that drive many `Document`s concurrently (e.g. servers rendering documents per request via `tokio::task::spawn_blocking`) need to disable parallel traversal to stay correct. The previous hardcoded `layout.threads = -1` (auto) gave them no way to do so. Add an additive `stylo_thread_count: Option<i32>` field to `DocumentConfig`. `None` preserves the historical default; embedders opt into serial traversal with `Some(1)`. Documents the one-shot nature of `STYLE_THREAD_POOL` initialisation in the rustdoc so callers aren't surprised that only the first `Document` constructed in the process gets to choose the value.
72757eb to
eb35068
Compare
Member
|
I think I'm going to use the approach in #437 instead, as it has the nice property of being per-Document. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stylo's
STYLE_THREAD_POOLis a process-globalLazyLock<ThreadPool>. Each rayon worker thread in that pool holds a thread-localAtomicRefCell<SharingCache>borrowed mutably for the entire duration of a style traversal landing on that worker (seesharing/mod.rs:563-619). The architectural contract is "only one style traversal at a time touches the global pool" — true for Servo and Gecko (one layout thread per Document) but false for any embedder driving multipleDocuments concurrently. When a second traversal lands on a worker mid-traversal of the first, it triesborrow_muton an already-borrowed cell and panics withalready mutably borrowed.This PR adds an additive escape hatch on
DocumentConfigso embedders can pin style traversal to a single thread (the calling thread, where eachDocument's stylo work has uniquely-owned thread-locals). It does not change blitz's default behaviour.Repro
With this PR:
Changes
packages/blitz-dom/src/config.rspub stylo_thread_count: Option<i32>field onDocumentConfig, with rustdoc covering the rationale and the "one-shot" gotcha (STYLE_THREAD_POOLis aLazyLock; the firstDocument::newcall's value wins for the whole process).default_stylo_thread_count_is_none(regression-pin:Noneis load-bearing for the historical default).packages/blitz-dom/src/document.rs:353set_pref!("layout.threads", -1)toset_pref!("layout.threads", config.stylo_thread_count.unwrap_or(-1)).unwrap_or(-1)preserves the historical auto-detect default for every existing caller.Compatibility
Fully additive: new field on a struct that already derives
Default, so every existing call site is unaffected. Default valueNonepreserves the historical-1(auto, capped at 6) behaviour.Test plan for reviewers
cargo test --workspace).cargo fmt --check,cargo clippy --workspace -- -D warnings,RUSTDOCFLAGS="-D warnings" cargo doc -p blitz-dom --no-deps,cargo build --workspaceon MSRV 1.92 — all unchanged from pre-PR baseline.main(panics) vs. this PR withstylo_thread_count: Some(1)(no panic).