From 0481477f35f75de7f77e42ada39e0ffbad3a1fc9 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Sat, 30 Aug 2025 15:47:26 -0400 Subject: [PATCH] script_bindings: Remove Cell wrapper from thread-local RootCollection. (#39043) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This doesn't appear to make a big difference in speedometer results, but this removes some code from the hot path of creating DomRoot values. Before: `Score: 30.381097406624708 ± 2.0393225244958018` After: `Score: 30.344639420871395 ± 1.9359337921154696` Testing: Existing WPT coverage Signed-off-by: Josh Matthews --- components/script/dom/bindings/root.rs | 16 ---------------- .../script/dom/dedicatedworkerglobalscope.rs | 5 +---- .../script/dom/serviceworkerglobalscope.rs | 5 +---- components/script/dom/worklet.rs | 4 +--- components/script/script_thread.rs | 4 +--- components/script_bindings/root.rs | 12 +++++------- 6 files changed, 9 insertions(+), 37 deletions(-) diff --git a/components/script/dom/bindings/root.rs b/components/script/dom/bindings/root.rs index e03c7392471..8bbf0d253a7 100644 --- a/components/script/dom/bindings/root.rs +++ b/components/script/dom/bindings/root.rs @@ -27,7 +27,6 @@ use std::cell::{OnceCell, UnsafeCell}; use std::default::Default; use std::hash::{Hash, Hasher}; -use std::marker::PhantomData; use std::{mem, ptr}; use js::jsapi::{Heap, JSObject, JSTracer, Value}; @@ -43,21 +42,6 @@ use crate::dom::bindings::reflector::DomObject; use crate::dom::bindings::trace::JSTraceable; use crate::dom::node::Node; -pub(crate) struct ThreadLocalStackRoots<'a>(PhantomData<&'a u32>); - -impl<'a> ThreadLocalStackRoots<'a> { - pub(crate) fn new(roots: &'a RootCollection) -> Self { - STACK_ROOTS.with(|r| r.set(Some(roots))); - ThreadLocalStackRoots(PhantomData) - } -} - -impl Drop for ThreadLocalStackRoots<'_> { - fn drop(&mut self) { - STACK_ROOTS.with(|r| r.set(None)); - } -} - pub(crate) trait ToLayout { /// Returns `LayoutDom` containing the same pointer. /// diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 5c85312413a..432abc3686e 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -40,7 +40,7 @@ use crate::dom::bindings::codegen::Bindings::WorkerBinding::WorkerType; use crate::dom::bindings::error::{ErrorInfo, ErrorResult}; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::reflector::DomGlobal; -use crate::dom::bindings::root::{DomRoot, RootCollection, ThreadLocalStackRoots}; +use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::str::DOMString; use crate::dom::bindings::structuredclone; use crate::dom::bindings::trace::{CustomTraceable, RootedTraceableBox}; @@ -396,9 +396,6 @@ impl DedicatedWorkerGlobalScope { WebViewId::install(webview_id); } - let roots = RootCollection::new(); - let _stack_roots = ThreadLocalStackRoots::new(&roots); - let WorkerScriptLoadOrigin { referrer_url, referrer_policy, diff --git a/components/script/dom/serviceworkerglobalscope.rs b/components/script/dom/serviceworkerglobalscope.rs index 283740d87da..62720e52c1e 100644 --- a/components/script/dom/serviceworkerglobalscope.rs +++ b/components/script/dom/serviceworkerglobalscope.rs @@ -35,7 +35,7 @@ use crate::dom::bindings::codegen::Bindings::ServiceWorkerGlobalScopeBinding; use crate::dom::bindings::codegen::Bindings::ServiceWorkerGlobalScopeBinding::ServiceWorkerGlobalScopeMethods; use crate::dom::bindings::codegen::Bindings::WorkerBinding::WorkerType; use crate::dom::bindings::inheritance::Castable; -use crate::dom::bindings::root::{DomRoot, RootCollection, ThreadLocalStackRoots}; +use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::str::DOMString; use crate::dom::bindings::structuredclone; use crate::dom::bindings::trace::CustomTraceable; @@ -317,9 +317,6 @@ impl ServiceWorkerGlobalScope { let context_for_interrupt = runtime.thread_safe_js_context(); let _ = context_sender.send(context_for_interrupt); - let roots = RootCollection::new(); - let _stack_roots = ThreadLocalStackRoots::new(&roots); - let WorkerScriptLoadOrigin { referrer_url, referrer_policy, diff --git a/components/script/dom/worklet.rs b/components/script/dom/worklet.rs index 650a101033c..6dc28120193 100644 --- a/components/script/dom/worklet.rs +++ b/components/script/dom/worklet.rs @@ -38,7 +38,7 @@ use crate::dom::bindings::error::Error; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::refcounted::TrustedPromise; use crate::dom::bindings::reflector::{DomGlobal, Reflector, reflect_dom_object}; -use crate::dom::bindings::root::{Dom, DomRoot, RootCollection, ThreadLocalStackRoots}; +use crate::dom::bindings::root::{Dom, DomRoot}; use crate::dom::bindings::str::USVString; use crate::dom::bindings::trace::{CustomTraceable, JSTraceable, RootedTraceableBox}; use crate::dom::csp::Violation; @@ -495,8 +495,6 @@ impl WorkletThread { // TODO: configure the JS runtime (e.g. discourage GC, encourage agressive JIT) debug!("Initializing worklet thread."); thread_state::initialize(ThreadState::SCRIPT | ThreadState::IN_WORKER); - let roots = RootCollection::new(); - let _stack_roots = ThreadLocalStackRoots::new(&roots); let mut thread = RootedTraceableBox::new(WorkletThread { role, control_receiver, diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 9e2388d2b5a..abde3d68851 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -111,7 +111,7 @@ use crate::dom::bindings::conversions::{ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::refcounted::Trusted; use crate::dom::bindings::reflector::DomGlobal; -use crate::dom::bindings::root::{Dom, DomRoot, RootCollection, ThreadLocalStackRoots}; +use crate::dom::bindings::root::{Dom, DomRoot}; use crate::dom::bindings::settings_stack::AutoEntryScript; use crate::dom::bindings::str::DOMString; use crate::dom::bindings::trace::{HashMapTracedValues, JSTraceable}; @@ -415,8 +415,6 @@ impl ScriptThreadFactory for ScriptThread { thread_state::initialize(ThreadState::SCRIPT | ThreadState::LAYOUT); PipelineNamespace::install(state.pipeline_namespace_id); WebViewId::install(state.webview_id); - let roots = RootCollection::new(); - let _stack_roots = ThreadLocalStackRoots::new(&roots); let memory_profiler_sender = state.memory_profiler_sender.clone(); let in_progress_load = InProgressLoad::new( diff --git a/components/script_bindings/root.rs b/components/script_bindings/root.rs index 1f635c4df83..1f72e45652d 100644 --- a/components/script_bindings/root.rs +++ b/components/script_bindings/root.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use std::cell::{Cell, UnsafeCell}; +use std::cell::UnsafeCell; use std::hash::{Hash, Hasher}; use std::ops::Deref; use std::{fmt, mem, ptr}; @@ -40,9 +40,8 @@ where unsafe fn add_to_root_list(object: *const dyn JSTraceable) -> *const RootCollection { assert_in_script(); STACK_ROOTS.with(|root_list| { - let root_list = unsafe { &*root_list.get().unwrap() }; unsafe { root_list.root(object) }; - root_list + root_list as *const _ }) } @@ -390,8 +389,7 @@ pub struct RootCollection { impl RootCollection { /// Create an empty collection of roots #[allow(clippy::new_without_default)] - pub fn new() -> RootCollection { - assert_in_script(); + pub const fn new() -> RootCollection { RootCollection { roots: UnsafeCell::new(vec![]), } @@ -419,7 +417,7 @@ impl RootCollection { } } -thread_local!(pub static STACK_ROOTS: Cell> = const { Cell::new(None) }); +thread_local!(pub static STACK_ROOTS: RootCollection = const { RootCollection::new() }); /// SM Callback that traces the rooted reflectors /// @@ -428,7 +426,7 @@ thread_local!(pub static STACK_ROOTS: Cell> = cons pub unsafe fn trace_roots(tracer: *mut JSTracer) { trace!("tracing stack roots"); STACK_ROOTS.with(|collection| { - let collection = unsafe { &*(*collection.get().unwrap()).roots.get() }; + let collection = unsafe { &*collection.roots.get() }; for root in collection { unsafe { (**root).trace(tracer);