diff --git a/Cargo.lock b/Cargo.lock index 70122773a..ee8b90330 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -361,6 +361,16 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "borsh" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "553c5d846a6ba5150c65e3b1b8ec073bcf1abc20f9b7220de384a4443ea4e20a" +dependencies = [ + "bytes", + "cfg_aliases", +] + [[package]] name = "boxed_error" version = "0.2.3" @@ -474,6 +484,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" + [[package]] name = "charset" version = "0.1.5" @@ -2483,6 +2499,7 @@ dependencies = [ "selectors", "serde_json", "servo_arc", + "smol_str", "thin-vec", "url", ] @@ -4527,6 +4544,16 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" +[[package]] +name = "smol_str" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4aaa7368fcf4852a4c2dd92df0cace6a71f2091ca0a23391ce7f3a31833f1523" +dependencies = [ + "borsh", + "serde_core", +] + [[package]] name = "socket2" version = "0.6.3" diff --git a/moli-dom/Cargo.toml b/moli-dom/Cargo.toml index 188044b6f..c3c92a4f3 100644 --- a/moli-dom/Cargo.toml +++ b/moli-dom/Cargo.toml @@ -13,6 +13,7 @@ percent-encoding = "2.3" selectors = "0.40" serde_json = "1.0.145" servo_arc = "0.4.3" +smol_str = "0.3.6" thin-vec = "0.2.14" url = "2.5.7" diff --git a/moli-dom/src/native/host/mutation/tree.rs b/moli-dom/src/native/host/mutation/tree.rs index bd360d3fa..9dc0d87e7 100644 --- a/moli-dom/src/native/host/mutation/tree.rs +++ b/moli-dom/src/native/host/mutation/tree.rs @@ -544,6 +544,11 @@ impl DomHost { self.dom.text_content(handle) } + pub fn shared_text_content(&self, handle: DomHandle) -> Option> { + self.node(handle) + .map(|node| node.shared_text_content(&self.dom)) + } + pub fn inner_html(&self, handle: DomHandle) -> Option { self.dom.inner_html(handle) } diff --git a/moli-dom/src/native/mod.rs b/moli-dom/src/native/mod.rs index bd30e0fd9..253e6a1f4 100644 --- a/moli-dom/src/native/mod.rs +++ b/moli-dom/src/native/mod.rs @@ -402,7 +402,7 @@ impl NativeDom { data: &str, ) -> NativeNodeId { self.create_node( - NodeData::Text(Text::new(data.to_owned())), + NodeData::Text(Text::new(data)), Some(owner_document), false, false, @@ -908,7 +908,7 @@ mod tests { "DocumentType grew to {} bytes", size_of::() ); - assert_eq!(size_of::(), 16); + assert_eq!(size_of::(), 24); assert_eq!(size_of::(), 16); assert_eq!(size_of::(), 16); assert_eq!(size_of::(), 32); diff --git a/moli-dom/src/native/node/mod.rs b/moli-dom/src/native/node/mod.rs index 556d34d61..33a548094 100644 --- a/moli-dom/src/native/node/mod.rs +++ b/moli-dom/src/native/node/mod.rs @@ -6,6 +6,7 @@ pub use types::{CDataSection, Comment, ProcessingInstruction, Text}; use std::fmt; use std::num::NonZeroU32; +use std::sync::Arc; use super::NativeDom; use super::element::Element; @@ -509,6 +510,42 @@ impl Node { }) } + pub(crate) fn shared_text_content(&self, dom: &NativeDom) -> Arc { + match self.data() { + NodeData::Text(text) => return text.shared_data(), + NodeData::CDataSection(cdata) => return Arc::from(cdata.data()), + NodeData::Comment(comment) => return Arc::from(comment.data()), + NodeData::ProcessingInstruction(processing_instruction) => { + return Arc::from(processing_instruction.data()); + } + NodeData::DocumentType(_) => return Arc::from(""), + NodeData::Document(_) | NodeData::Element(_) | NodeData::DocumentFragment(_) => {} + } + + let mut only_text = None; + let mut stack = dom.child_ids_reversed(self.id()).collect::>(); + while let Some(node_id) = stack.pop() { + let Some(node) = dom.node(node_id) else { + continue; + }; + match node.data() { + NodeData::Text(text) if only_text.is_none() => only_text = Some(text), + NodeData::Text(_) | NodeData::CDataSection(_) => { + return Arc::from(self.text_content(dom)); + } + NodeData::Document(_) | NodeData::Element(_) | NodeData::DocumentFragment(_) => { + stack.extend(dom.child_ids_reversed(node_id)); + } + NodeData::Comment(_) + | NodeData::ProcessingInstruction(_) + | NodeData::DocumentType(_) => {} + } + } + only_text + .map(Text::shared_data) + .unwrap_or_else(|| Arc::from("")) + } + pub fn metadata(&self) -> LiveDomNodeMetadata { match self.data() { NodeData::Document(_) => LiveDomNodeMetadata { diff --git a/moli-dom/src/native/node/types.rs b/moli-dom/src/native/node/types.rs index 9be7c40af..37d4b3eb8 100644 --- a/moli-dom/src/native/node/types.rs +++ b/moli-dom/src/native/node/types.rs @@ -1,21 +1,48 @@ +use smol_str::SmolStr; +use std::sync::Arc; + #[derive(Debug, Clone)] pub struct Text { - data: Box, + data: SmolStr, } impl Text { - pub fn new(data: String) -> Self { - Self { - data: data.into_boxed_str(), - } + pub fn new(data: impl Into) -> Self { + Self { data: data.into() } } pub fn data(&self) -> &str { - &self.data + self.data.as_str() } - pub fn set_data(&mut self, data: impl Into) { - self.data = data.into().into_boxed_str(); + pub(crate) fn shared_data(&self) -> Arc { + Arc::from(self.data.clone()) + } + + pub fn set_data(&mut self, data: impl Into>) { + self.data = SmolStr::from(data.into()); + } +} + +#[cfg(test)] +mod tests { + use super::Text; + use std::sync::Arc; + + #[test] + fn text_inlines_short_data() { + let text = Text::new("short text"); + + assert!(!text.data.is_heap_allocated()); + } + + #[test] + fn text_shares_long_data_when_requested() { + let text = Text::new("x".repeat(64)); + + let first = text.shared_data(); + let second = text.shared_data(); + assert!(Arc::ptr_eq(&first, &second)); } } diff --git a/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs b/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs index 0c35bde13..bfd62502e 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::{collections::HashSet, sync::Arc}; use super::{ JsContextHost, @@ -873,13 +873,13 @@ impl JsContextHost { } pub(crate) fn sync_owner_style_sheet_text(&mut self, owner: DomHandle) { - let css_text = self.dom_host().text_content(owner).unwrap_or_default(); + let css_text = self + .dom_host() + .shared_text_content(owner) + .unwrap_or_else(|| Arc::from("")); let dom_host = self.dom_host() as *const _; - self.style_engine.sync_owner_style_sheet_text_with_host( - unsafe { &*dom_host }, - owner, - css_text, - ); + self.style_engine + .sync_owner_style_sheet_text_backing_with_host(unsafe { &*dom_host }, owner, css_text); self.install_owner_live_stylesheet(owner); } diff --git a/moli-renderer-v8/src/style_engine/source/mod.rs b/moli-renderer-v8/src/style_engine/source/mod.rs index 77dfad212..69b10bc50 100644 --- a/moli-renderer-v8/src/style_engine/source/mod.rs +++ b/moli-renderer-v8/src/style_engine/source/mod.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::{collections::HashSet, sync::Arc}; pub(super) mod adopted; pub(super) mod imports; @@ -160,21 +160,31 @@ impl MoliStyleEngine { .tracks_document(document) } + #[cfg(test)] pub(crate) fn set_owner_style_sheet_text_with_host( &mut self, host: &DomHost, owner: DomHandle, css_text: String, + ) { + self.set_owner_style_sheet_text_backing_with_host(host, owner, css_text.into()); + } + + fn set_owner_style_sheet_text_backing_with_host( + &mut self, + host: &DomHost, + owner: DomHandle, + css_text: Arc, ) { let parser_base = stylesheet_source_base_url(host, owner); self.set_owner_style_sheet_source_with_parser_base(host, owner, css_text, parser_base); } - pub(crate) fn sync_owner_style_sheet_text_with_host( + pub(crate) fn sync_owner_style_sheet_text_backing_with_host( &mut self, host: &DomHost, owner: DomHandle, - css_text: String, + css_text: Arc, ) { if self.owner_document_world(host, owner).is_some_and(|world| { world @@ -184,14 +194,14 @@ impl MoliStyleEngine { }) { return; } - self.set_owner_style_sheet_text_with_host(host, owner, css_text); + self.set_owner_style_sheet_text_backing_with_host(host, owner, css_text); } fn process_owner_style_sheet_text_with_host( &mut self, host: &DomHost, owner: DomHandle, - css_text: String, + css_text: Arc, ) { let Some(document) = owner_document_for_source_owner(host, owner) else { return; @@ -214,7 +224,7 @@ impl MoliStyleEngine { &mut self, host: &DomHost, owner: DomHandle, - css_text: String, + css_text: Arc, parser_base: url::Url, ) { let Some(document) = owner_document_for_source_owner(host, owner) else { @@ -518,7 +528,8 @@ impl MoliStyleEngine { self.process_owner_style_sheet_text_with_host( host, owner, - host.text_content(owner).unwrap_or_default(), + host.shared_text_content(owner) + .unwrap_or_else(|| Arc::from("")), ); } else if matches!( change.kind(), diff --git a/moli-renderer-v8/src/style_engine/source/shared_cache.rs b/moli-renderer-v8/src/style_engine/source/shared_cache.rs index c12f03956..d770e27ee 100644 --- a/moli-renderer-v8/src/style_engine/source/shared_cache.rs +++ b/moli-renderer-v8/src/style_engine/source/shared_cache.rs @@ -49,13 +49,19 @@ struct SharedStyleSourceCache { pub(super) fn shared_style_source_contents( css_text: String, base_url: url::Url, +) -> Arc { + shared_style_source_contents_from_shared(css_text.into(), base_url) +} + +pub(super) fn shared_style_source_contents_from_shared( + css_text: Arc, + base_url: url::Url, ) -> Arc { let key = SharedStyleSourceCacheKey::new(&css_text, &base_url); if let Some(cached) = CACHE.lock().lookup(&key, &css_text, &base_url) { return cached; } - let css_text = Arc::::from(css_text); let metadata = style_source_metadata_for_css_text(&css_text, &base_url); let source = Arc::new(SharedStyleSourceContents { source_metadata: SharedStyleSourceMetadata::from_metadata( @@ -288,6 +294,17 @@ mod tests { assert_eq!(cache.retained_bytes, 0); } + #[test] + fn shared_input_becomes_the_retained_css_text_backing() { + let css_text = Arc::::from(".shared-input { color: green; }"); + let source = shared_style_source_contents_from_shared( + Arc::clone(&css_text), + url::Url::parse("https://shared-input.test/style.css").expect("valid base URL"), + ); + + assert!(Arc::ptr_eq(&css_text, &source.css_text_handle())); + } + #[test] fn weak_cache_evicts_oldest_index_to_byte_budget() { let mut cache = SharedStyleSourceCache::with_retained_bytes_limit(2 * ENTRY_RETAINED_BYTES); diff --git a/moli-renderer-v8/src/style_engine/source/store.rs b/moli-renderer-v8/src/style_engine/source/store.rs index 52abf400b..feb297961 100644 --- a/moli-renderer-v8/src/style_engine/source/store.rs +++ b/moli-renderer-v8/src/style_engine/source/store.rs @@ -13,7 +13,10 @@ use super::super::{ source_key::{StyleSourceKey, StyleSourceSetKey}, }; use super::imports::stylesheet_top_level_import_urls; -use super::shared_cache::{SharedStyleSourceContents, shared_style_source_contents}; +use super::shared_cache::{ + SharedStyleSourceContents, shared_style_source_contents, + shared_style_source_contents_from_shared, +}; use crate::{ document_runtime::DomHandle, protocol_types::EmulatedMediaOverrides, style_engine::StyleViewport, @@ -92,6 +95,15 @@ pub(crate) struct OwnerStyleSheetSource { impl StyloStylesheetSource { pub(crate) fn new(css_text: String, base_url: url::Url) -> Self { let shared = shared_style_source_contents(css_text, base_url); + Self::from_shared_contents(shared) + } + + fn from_shared_text(css_text: StdArc, base_url: url::Url) -> Self { + let shared = shared_style_source_contents_from_shared(css_text, base_url); + Self::from_shared_contents(shared) + } + + fn from_shared_contents(shared: StdArc) -> Self { let cache_key = StyleSourceKey::from_css_fingerprint(shared.css_fingerprint(), shared.base_url()); let base_url = shared.base_url_handle(); @@ -542,8 +554,22 @@ impl StylesheetFontFaceDescriptor { } impl OwnerStyleSheetSource { + #[cfg(test)] pub(crate) fn new(owner: DomHandle, css_text: String, parser_base: url::Url) -> Self { let source = StyloStylesheetSource::new(css_text, parser_base); + Self::from_source(owner, source) + } + + pub(crate) fn from_shared_text( + owner: DomHandle, + css_text: StdArc, + parser_base: url::Url, + ) -> Self { + let source = StyloStylesheetSource::from_shared_text(css_text, parser_base); + Self::from_source(owner, source) + } + + fn from_source(owner: DomHandle, source: StyloStylesheetSource) -> Self { let processing_contents = source .processing_contents() .expect("owner processing source must remain text-backed"); diff --git a/moli-renderer-v8/src/style_engine/source_owner_text.rs b/moli-renderer-v8/src/style_engine/source_owner_text.rs index 7bc5b1a2f..99fabcebc 100644 --- a/moli-renderer-v8/src/style_engine/source_owner_text.rs +++ b/moli-renderer-v8/src/style_engine/source_owner_text.rs @@ -94,7 +94,7 @@ impl OwnerStyleSheetSources { pub(super) fn set_source( &mut self, owner: DomHandle, - css_text: String, + css_text: Arc, parser_base: url::Url, ) -> bool { self.cssom_authoritative_owners.remove(&owner); @@ -105,7 +105,11 @@ impl OwnerStyleSheetSources { }) { return false; } - let processing_source = Arc::new(OwnerStyleSheetSource::new(owner, css_text, parser_base)); + let processing_source = Arc::new(OwnerStyleSheetSource::from_shared_text( + owner, + css_text, + parser_base, + )); self.sources_by_owner.insert( owner, InstalledOwnerStyleSheet::from_processing_source(processing_source), @@ -116,11 +120,15 @@ impl OwnerStyleSheetSources { pub(super) fn replace_processed_source( &mut self, owner: DomHandle, - css_text: String, + css_text: Arc, parser_base: url::Url, ) { self.cssom_authoritative_owners.remove(&owner); - let processing_source = Arc::new(OwnerStyleSheetSource::new(owner, css_text, parser_base)); + let processing_source = Arc::new(OwnerStyleSheetSource::from_shared_text( + owner, + css_text, + parser_base, + )); self.sources_by_owner.insert( owner, InstalledOwnerStyleSheet::from_processing_source(processing_source), diff --git a/moli-renderer-v8/src/style_engine/tests/lifecycle.rs b/moli-renderer-v8/src/style_engine/tests/lifecycle.rs index ee648ef7e..9f24eb9e2 100644 --- a/moli-renderer-v8/src/style_engine/tests/lifecycle.rs +++ b/moli-renderer-v8/src/style_engine/tests/lifecycle.rs @@ -1755,8 +1755,8 @@ fn owner_style_sheet_sources_share_one_cached_text_allocation() { let mut owner_sources = super::source_owner_text::OwnerStyleSheetSources::default(); let base_url = url::Url::parse("https://example.test/").expect("valid test url"); let css_text = ".card:has(.item) { color: red; }"; - owner_sources.set_source(first_owner, css_text.to_owned(), base_url.clone()); - owner_sources.set_source(second_owner, css_text.to_owned(), base_url); + owner_sources.set_source(first_owner, css_text.into(), base_url.clone()); + owner_sources.set_source(second_owner, css_text.into(), base_url); let first = owner_sources .source(first_owner) @@ -1852,7 +1852,7 @@ fn linked_source_store_lifecycle_records_drive_retained_record_construction() { let mut owner_sources = super::source_owner_text::OwnerStyleSheetSources::default(); owner_sources.set_source( style, - ".owner { color: green; }".to_owned(), + ".owner { color: green; }".into(), url::Url::parse("https://example.test/").unwrap(), ); let linked_url = url::Url::parse("https://example.test/linked.css").unwrap();