From 922d4b83de2ce2865a5bf113f4fea513e1f98395 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender <55576758+jschwe@users.noreply.github.com> Date: Wed, 25 Jun 2025 08:47:20 +0200 Subject: [PATCH] Use FnvHashmap for LiveDOMReferences (#37673) These maps are keyed on pointers, so using FnvHashMap should be faster and we don't need the collision resistance properties of the default hasher. These maps showed up as hot when profiling the testcase from https://github.com/servo/servo/issues/37223#issuecomment-3000705438. Switching the hashing algorithm reduces the time spent hashing, but overall that is still only a fractional part of the testcase. Testing: Functionality unchanged. Overall the performance change is to small to show up in any of our automated performance tests. When comparing the perf profile of the above linked testcase, one should see a reduction of time spent hashing (inverse call stack) Signed-off-by: Jonathan Schwender --- components/script/dom/bindings/refcounted.rs | 12 ++++++------ components/script/dom/document.rs | 5 +++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/components/script/dom/bindings/refcounted.rs b/components/script/dom/bindings/refcounted.rs index 1685024593a..d44450d3ae9 100644 --- a/components/script/dom/bindings/refcounted.rs +++ b/components/script/dom/bindings/refcounted.rs @@ -24,12 +24,12 @@ use std::cell::RefCell; use std::collections::hash_map::Entry::{Occupied, Vacant}; -use std::collections::hash_map::HashMap; use std::hash::Hash; use std::marker::PhantomData; use std::rc::Rc; use std::sync::{Arc, Weak}; +use fnv::FnvHashMap; use js::jsapi::JSTracer; use script_bindings::script_runtime::CanGc; @@ -230,8 +230,8 @@ impl Clone for Trusted { #[cfg_attr(crown, allow(crown::unrooted_must_root))] pub(crate) struct LiveDOMReferences { // keyed on pointer to Rust DOM object - reflectable_table: RefCell>>, - promise_table: RefCell>>>, + reflectable_table: RefCell>>, + promise_table: RefCell>>>, } impl LiveDOMReferences { @@ -239,8 +239,8 @@ impl LiveDOMReferences { pub(crate) fn initialize() { LIVE_REFERENCES.with(|r| { *r.borrow_mut() = Some(LiveDOMReferences { - reflectable_table: RefCell::new(HashMap::new()), - promise_table: RefCell::new(HashMap::new()), + reflectable_table: RefCell::new(FnvHashMap::default()), + promise_table: RefCell::new(FnvHashMap::default()), }) }); } @@ -289,7 +289,7 @@ impl LiveDOMReferences { } /// Remove null entries from the live references table -fn remove_nulls(table: &mut HashMap>) { +fn remove_nulls(table: &mut FnvHashMap>) { let to_remove: Vec = table .iter() .filter(|&(_, value)| Weak::upgrade(value).is_none()) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 22b3bc24e89..20b1456a4d1 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -35,6 +35,7 @@ use embedder_traits::{ }; use encoding_rs::{Encoding, UTF_8}; use euclid::default::{Point2D, Rect, Size2D}; +use fnv::FnvHashMap; use html5ever::{LocalName, Namespace, QualName, local_name, ns}; use hyper_serde::Serde; use ipc_channel::ipc; @@ -380,7 +381,7 @@ pub(crate) struct Document { appropriate_template_contents_owner_document: MutNullableDom, /// Information on elements needing restyle to ship over to layout when the /// time comes. - pending_restyles: DomRefCell, NoTrace>>, + pending_restyles: DomRefCell, NoTrace>>, /// This flag will be true if the `Document` needs to be painted again /// during the next full layout attempt due to some external change such as /// the web view changing size, or because the previous layout was only for @@ -4162,7 +4163,7 @@ impl Document { current_parser: Default::default(), base_element: Default::default(), appropriate_template_contents_owner_document: Default::default(), - pending_restyles: DomRefCell::new(HashMap::new()), + pending_restyles: DomRefCell::new(FnvHashMap::default()), needs_paint: Cell::new(false), active_touch_points: DomRefCell::new(Vec::new()), dom_interactive: Cell::new(Default::default()),