From 636e7211e001058e36487fd3f763b2f93afc06ed Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 20 Aug 2025 04:43:58 -0400 Subject: [PATCH] script: Measure heap usage of various ignored fields (#38791) These changes allow using MallocSizeOf/`#[conditional_malloc_size_of]` on WebIDL callback values, and then fix a grab bag of places in the script crate that previously ignored those values. There are also some commits removing ignored fields that involved Arc/Rc that are not WebIDL callbacks, since they are now easier to support with the `#[conditional_malloc_size_of]` attribute. Testing: Manual testing on about:memory for servo.org. --------- Signed-off-by: Josh Matthews --- components/script/clipboard_provider.rs | 2 ++ .../script/dom/bindings/buffer_source.rs | 2 +- .../script/dom/customelementregistry.rs | 22 +++++++++---------- components/script/dom/eventtarget.rs | 10 ++++----- components/script/dom/htmlinputelement.rs | 1 - components/script/dom/htmlmediaelement.rs | 12 ++++++---- components/script/dom/htmltextareaelement.rs | 1 - components/script/dom/raredata.rs | 2 +- .../dom/readablestreamdefaultcontroller.rs | 6 ++--- components/script/microtask.rs | 4 ++-- components/script/task.rs | 2 +- components/script/timers.rs | 3 +-- components/script_bindings/callback.rs | 8 ++++--- components/script_bindings/codegen/codegen.py | 2 +- .../constellation/from_script_message.rs | 3 ++- components/shared/layout/lib.rs | 2 +- 16 files changed, 44 insertions(+), 38 deletions(-) diff --git a/components/script/clipboard_provider.rs b/components/script/clipboard_provider.rs index 4c0f5467be6..4c814942f60 100644 --- a/components/script/clipboard_provider.rs +++ b/components/script/clipboard_provider.rs @@ -6,6 +6,7 @@ use base::id::WebViewId; use constellation_traits::{ScriptToConstellationChan, ScriptToConstellationMessage}; use embedder_traits::EmbedderMsg; use ipc_channel::ipc::channel; +use malloc_size_of_derive::MallocSizeOf; /// A trait which abstracts access to the embedder's clipboard in order to allow unit /// testing clipboard-dependent parts of `script`. @@ -16,6 +17,7 @@ pub trait ClipboardProvider { fn set_text(&mut self, _: String); } +#[derive(MallocSizeOf)] pub(crate) struct EmbedderClipboardProvider { pub constellation_sender: ScriptToConstellationChan, pub webview_id: WebViewId, diff --git a/components/script/dom/bindings/buffer_source.rs b/components/script/dom/bindings/buffer_source.rs index 551cf044eb2..d9406903390 100644 --- a/components/script/dom/bindings/buffer_source.rs +++ b/components/script/dom/bindings/buffer_source.rs @@ -823,7 +823,7 @@ pub(crate) fn create_array_buffer_with_size( #[cfg(feature = "webgpu")] #[derive(JSTraceable, MallocSizeOf)] pub(crate) struct DataBlock { - #[ignore_malloc_size_of = "Arc"] + #[conditional_malloc_size_of] data: Arc>, /// Data views (mutable subslices of data) data_views: Vec, diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index c7f9797157d..c0595dad429 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -614,28 +614,28 @@ impl CustomElementRegistryMethods for CustomElementRegistr #[derive(Clone, JSTraceable, MallocSizeOf)] pub(crate) struct LifecycleCallbacks { - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] connected_callback: Option>, - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] disconnected_callback: Option>, - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] adopted_callback: Option>, - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] attribute_changed_callback: Option>, - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] form_associated_callback: Option>, - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] form_reset_callback: Option>, - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] form_disabled_callback: Option>, - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] form_state_restore_callback: Option>, } @@ -657,7 +657,7 @@ pub(crate) struct CustomElementDefinition { pub(crate) local_name: LocalName, /// - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] pub(crate) constructor: Rc, /// @@ -988,9 +988,9 @@ pub(crate) fn try_upgrade_element(element: &Element) { #[derive(JSTraceable, MallocSizeOf)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub(crate) enum CustomElementReaction { - Upgrade(#[ignore_malloc_size_of = "Rc"] Rc), + Upgrade(#[conditional_malloc_size_of] Rc), Callback( - #[ignore_malloc_size_of = "Rc"] Rc, + #[conditional_malloc_size_of] Rc, #[ignore_malloc_size_of = "mozjs"] Box<[Heap]>, ), } diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index eb96a910c1e..8a614c269a3 100644 --- a/components/script/dom/eventtarget.rs +++ b/components/script/dom/eventtarget.rs @@ -163,11 +163,11 @@ static CONTENT_EVENT_HANDLER_NAMES: [&str; 83] = [ #[derive(Clone, JSTraceable, MallocSizeOf, PartialEq)] #[allow(clippy::enum_variant_names)] pub(crate) enum CommonEventHandler { - EventHandler(#[ignore_malloc_size_of = "Rc"] Rc), + EventHandler(#[conditional_malloc_size_of] Rc), - ErrorEventHandler(#[ignore_malloc_size_of = "Rc"] Rc), + ErrorEventHandler(#[conditional_malloc_size_of] Rc), - BeforeUnloadEventHandler(#[ignore_malloc_size_of = "Rc"] Rc), + BeforeUnloadEventHandler(#[conditional_malloc_size_of] Rc), } impl CommonEventHandler { @@ -231,7 +231,7 @@ fn get_compiled_handler( #[derive(Clone, JSTraceable, MallocSizeOf, PartialEq)] enum EventListenerType { - Additive(#[ignore_malloc_size_of = "Rc"] Rc), + Additive(#[conditional_malloc_size_of] Rc), Inline(RefCell), } @@ -439,7 +439,7 @@ impl std::cmp::PartialEq for EventListenerEntry { #[derive(Clone, JSTraceable, MallocSizeOf)] /// A mix of potentially uncompiled and compiled event listeners of the same type. pub(crate) struct EventListeners( - #[ignore_malloc_size_of = "Rc"] Vec>>, + #[conditional_malloc_size_of] Vec>>, ); impl Deref for EventListeners { diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index ae66ded7739..6ffcd5ad088 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -361,7 +361,6 @@ pub(crate) struct HTMLInputElement { size: Cell, maxlength: Cell, minlength: Cell, - #[ignore_malloc_size_of = "TextInput contains an IPCSender which cannot be measured"] #[no_trace] textinput: DomRefCell>, // https://html.spec.whatwg.org/multipage/#concept-input-value-dirty-flag diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 8026f30136b..c1ac1bf7ed2 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -111,13 +111,17 @@ static MEDIA_CONTROL_CSS: &str = include_str!("../resources/media-controls.css") /// A JS file to control the media controls. static MEDIA_CONTROL_JS: &str = include_str!("../resources/media-controls.js"); -#[derive(PartialEq)] +#[derive(MallocSizeOf, PartialEq)] enum FrameStatus { Locked, Unlocked, } -struct FrameHolder(FrameStatus, VideoFrame); +#[derive(MallocSizeOf)] +struct FrameHolder( + FrameStatus, + #[ignore_malloc_size_of = "defined in servo-media"] VideoFrame, +); impl FrameHolder { fn new(frame: VideoFrame) -> FrameHolder { @@ -159,6 +163,7 @@ impl FrameHolder { } } +#[derive(MallocSizeOf)] pub(crate) struct MediaFrameRenderer { player_id: Option, compositor_api: CrossProcessCompositorApi, @@ -389,7 +394,7 @@ pub(crate) struct HTMLMediaElement { #[ignore_malloc_size_of = "servo_media"] #[no_trace] player: DomRefCell>>>, - #[ignore_malloc_size_of = "Arc"] + #[conditional_malloc_size_of] #[no_trace] video_renderer: Arc>, #[ignore_malloc_size_of = "Arc"] @@ -417,7 +422,6 @@ pub(crate) struct HTMLMediaElement { #[no_trace] blob_url: DomRefCell>, /// - #[ignore_malloc_size_of = "Rc"] played: DomRefCell, // https://html.spec.whatwg.org/multipage/#dom-media-audiotracks audio_tracks_list: MutNullableDom, diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 6ce581972d4..43b5ec5a0ab 100644 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -50,7 +50,6 @@ use crate::textinput::{ #[dom_struct] pub(crate) struct HTMLTextAreaElement { htmlelement: HTMLElement, - #[ignore_malloc_size_of = "TextInput contains an IPCSender which cannot be measured"] #[no_trace] textinput: DomRefCell>, placeholder: DomRefCell, diff --git a/components/script/dom/raredata.rs b/components/script/dom/raredata.rs index 81df7649a46..5082c1d5878 100644 --- a/components/script/dom/raredata.rs +++ b/components/script/dom/raredata.rs @@ -66,7 +66,7 @@ pub(crate) struct ElementRareData { /// pub(crate) custom_element_reaction_queue: Vec, /// - #[ignore_malloc_size_of = "Rc"] + #[conditional_malloc_size_of] pub(crate) custom_element_definition: Option>, /// pub(crate) custom_element_state: CustomElementState, diff --git a/components/script/dom/readablestreamdefaultcontroller.rs b/components/script/dom/readablestreamdefaultcontroller.rs index 3be1fc48988..7cfcbfba7fe 100644 --- a/components/script/dom/readablestreamdefaultcontroller.rs +++ b/components/script/dom/readablestreamdefaultcontroller.rs @@ -115,17 +115,18 @@ impl Callback for StartAlgorithmRejectionHandler { } /// -#[derive(Debug, JSTraceable, PartialEq)] +#[derive(Debug, JSTraceable, MallocSizeOf, PartialEq)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub(crate) struct ValueWithSize { /// + #[ignore_malloc_size_of = "Heap is measured by mozjs"] pub(crate) value: Box>, /// pub(crate) size: f64, } /// -#[derive(Debug, JSTraceable, PartialEq)] +#[derive(Debug, JSTraceable, MallocSizeOf, PartialEq)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub(crate) enum EnqueuedValue { /// A value enqueued from Rust. @@ -191,7 +192,6 @@ fn is_non_negative_number(value: &EnqueuedValue) -> bool { #[derive(Default, JSTraceable, MallocSizeOf)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub(crate) struct QueueWithSizes { - #[ignore_malloc_size_of = "EnqueuedValue::Js"] queue: VecDeque, /// pub(crate) total_size: f64, diff --git a/components/script/microtask.rs b/components/script/microtask.rs index 3f5969e7b7f..4796c37c901 100644 --- a/components/script/microtask.rs +++ b/components/script/microtask.rs @@ -55,7 +55,7 @@ pub(crate) trait MicrotaskRunnable { /// A promise callback scheduled to run during the next microtask checkpoint (#4283). #[derive(JSTraceable, MallocSizeOf)] pub(crate) struct EnqueuedPromiseCallback { - #[ignore_malloc_size_of = "Rc has unclear ownership"] + #[conditional_malloc_size_of] pub(crate) callback: Rc, #[no_trace] pub(crate) pipeline: PipelineId, @@ -66,7 +66,7 @@ pub(crate) struct EnqueuedPromiseCallback { /// identical to EnqueuedPromiseCallback once it's on the queue #[derive(JSTraceable, MallocSizeOf)] pub(crate) struct UserMicrotask { - #[ignore_malloc_size_of = "Rc has unclear ownership"] + #[conditional_malloc_size_of] pub(crate) callback: Rc, #[no_trace] pub(crate) pipeline: PipelineId, diff --git a/components/script/task.rs b/components/script/task.rs index d149cadd8a6..aa977e687b7 100644 --- a/components/script/task.rs +++ b/components/script/task.rs @@ -118,7 +118,7 @@ impl fmt::Debug for dyn TaskBox { /// Encapsulated state required to create cancellable tasks from non-script threads. #[derive(Clone, Default, JSTraceable, MallocSizeOf)] pub(crate) struct TaskCanceller { - #[ignore_malloc_size_of = "This is difficult, because only one of them should be measured"] + #[conditional_malloc_size_of] pub(crate) cancelled: Arc, } diff --git a/components/script/timers.rs b/components/script/timers.rs index 271185b7740..b4b17ac8efa 100644 --- a/components/script/timers.rs +++ b/components/script/timers.rs @@ -381,7 +381,6 @@ struct JsTimerEntry { // TODO: Handle rooting during invocation when movable GC is turned on #[derive(JSTraceable, MallocSizeOf)] pub(crate) struct JsTimerTask { - #[ignore_malloc_size_of = "Because it is non-owning"] handle: JsTimerHandle, #[no_trace] source: TimerSource, @@ -409,7 +408,7 @@ pub(crate) enum TimerCallback { enum InternalTimerCallback { StringTimerCallback(DOMString), FunctionTimerCallback( - #[ignore_malloc_size_of = "Rc"] Rc, + #[conditional_malloc_size_of] Rc, #[ignore_malloc_size_of = "Rc"] Rc]>>, ), } diff --git a/components/script_bindings/callback.rs b/components/script_bindings/callback.rs index 3e09541c60e..75924406139 100644 --- a/components/script_bindings/callback.rs +++ b/components/script_bindings/callback.rs @@ -55,11 +55,13 @@ pub enum ExceptionHandling { /// A common base class for representing IDL callback function and /// callback interface types. -#[derive(JSTraceable)] +#[derive(JSTraceable, MallocSizeOf)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub struct CallbackObject { /// The underlying `JSObject`. + #[ignore_malloc_size_of = "measured by mozjs"] callback: Heap<*mut JSObject>, + #[ignore_malloc_size_of = "measured by mozjs"] permanent_js_root: Heap, /// The ["callback context"], that is, the global to use as incumbent @@ -147,7 +149,7 @@ pub trait CallbackContainer { } /// A common base class for representing IDL callback function types. -#[derive(JSTraceable, PartialEq)] +#[derive(JSTraceable, MallocSizeOf, PartialEq)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub struct CallbackFunction { object: CallbackObject, @@ -180,7 +182,7 @@ impl CallbackFunction { } /// A common base class for representing IDL callback interface types. -#[derive(JSTraceable, PartialEq)] +#[derive(JSTraceable, MallocSizeOf, PartialEq)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub struct CallbackInterface { object: CallbackObject, diff --git a/components/script_bindings/codegen/codegen.py b/components/script_bindings/codegen/codegen.py index 53f271b0264..50a89a7cad6 100644 --- a/components/script_bindings/codegen/codegen.py +++ b/components/script_bindings/codegen/codegen.py @@ -8057,7 +8057,7 @@ class CGCallback(CGClass): constructors=self.getConstructors(), methods=realMethods, templateSpecialization=['D: DomTypes'], - decorators="#[derive(JSTraceable, PartialEq)]\n" + decorators="#[derive(JSTraceable, MallocSizeOf, PartialEq)]\n" "#[cfg_attr(crown, allow(crown::unrooted_must_root))]\n" "#[cfg_attr(crown, crown::unrooted_must_root_lint::allow_unrooted_interior)]") diff --git a/components/shared/constellation/from_script_message.rs b/components/shared/constellation/from_script_message.rs index 1ca6aa8f9b6..1f96d933a93 100644 --- a/components/shared/constellation/from_script_message.rs +++ b/components/shared/constellation/from_script_message.rs @@ -23,6 +23,7 @@ use euclid::default::Size2D as UntypedSize2D; use http::{HeaderMap, Method}; use ipc_channel::Error as IpcError; use ipc_channel::ipc::{IpcReceiver, IpcSender}; +use malloc_size_of_derive::MallocSizeOf; use net_traits::policy_container::PolicyContainer; use net_traits::request::{Destination, InsecureRequestsPolicy, Referrer, RequestBody}; use net_traits::storage_thread::StorageType; @@ -42,7 +43,7 @@ use crate::{ }; /// A Script to Constellation channel. -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)] pub struct ScriptToConstellationChan { /// Sender for communicating with constellation thread. pub sender: IpcSender<(PipelineId, ScriptToConstellationMessage)>, diff --git a/components/shared/layout/lib.rs b/components/shared/layout/lib.rs index 68208d25cab..3b7ae70bdcc 100644 --- a/components/shared/layout/lib.rs +++ b/components/shared/layout/lib.rs @@ -175,7 +175,7 @@ pub struct PendingRasterizationImage { pub size: DeviceIntSize, } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, MallocSizeOf)] pub struct MediaFrame { pub image_key: webrender_api::ImageKey, pub width: i32,