From 54244797688fbeec85baac6fcfc9b71d555bf72c Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 21 Mar 2025 15:55:00 +0100 Subject: [PATCH] metrics: Simplify `ProgressiveWebMetrics` (#35985) Simply how `ProgressiveWebMetrics` works: 1. Keep only a single struct instead of one in layout and one script that both implement the `ProgressiveWebMetrics` trait. Since layout and script are the same thread these can now just be a single `ProgressiveWebMetrics` struct stored in script. 2. Have the compositor be responsible for informing the Constellation (which informs the ScripThread) about paint metrics. This makes communication flow one way and removes one dependency between the compositor and script (of two). 3. All units tests are moved into the `metrics` crate itself since there is only one struct there now. Signed-off-by: Martin Robinson --- Cargo.lock | 13 +- components/compositing/compositor.rs | 122 +++--- components/compositing/tracing.rs | 1 - components/compositing/webview.rs | 6 - components/constellation/constellation.rs | 97 ++--- components/constellation/pipeline.rs | 11 +- components/constellation/tracing.rs | 30 +- components/layout_2020/display_list/mod.rs | 21 +- components/layout_thread_2020/lib.rs | 25 +- components/metrics/lib.rs | 371 +++++++++--------- components/script/dom/document.rs | 77 ++-- components/script/messaging.rs | 7 +- components/script/script_thread.rs | 67 +--- .../shared/compositing/constellation_msg.rs | 10 + components/shared/compositing/lib.rs | 7 +- components/shared/profile/Cargo.toml | 2 + components/shared/profile/time.rs | 3 +- components/shared/script/lib.rs | 17 +- components/shared/script/script_msg.rs | 15 - components/shared/script_layout/lib.rs | 6 - components/shared/webrender/display_list.rs | 12 + python/servo/testing_commands.py | 1 + tests/unit/metrics/Cargo.toml | 20 - tests/unit/metrics/interactive_time.rs | 125 ------ tests/unit/metrics/lib.rs | 8 - tests/unit/metrics/paint_time.rs | 129 ------ 26 files changed, 416 insertions(+), 787 deletions(-) delete mode 100644 tests/unit/metrics/Cargo.toml delete mode 100644 tests/unit/metrics/interactive_time.rs delete mode 100644 tests/unit/metrics/lib.rs delete mode 100644 tests/unit/metrics/paint_time.rs diff --git a/Cargo.lock b/Cargo.lock index cafdb7cb79b..df13be0d61e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4612,17 +4612,6 @@ dependencies = [ "servo_url", ] -[[package]] -name = "metrics_tests" -version = "0.0.1" -dependencies = [ - "base", - "ipc-channel", - "metrics", - "profile_traits", - "servo_url", -] - [[package]] name = "mime" version = "0.3.17" @@ -5846,8 +5835,10 @@ dependencies = [ "crossbeam-channel", "ipc-channel", "log", + "malloc_size_of_derive", "serde", "servo_config", + "servo_malloc_size_of", "signpost", "strum_macros", "time", diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 4c8034aa883..d1abe4a8f62 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -18,7 +18,8 @@ use base::id::{PipelineId, WebViewId}; use base::{Epoch, WebRenderEpochToU16}; use bitflags::bitflags; use compositing_traits::{ - CompositionPipeline, CompositorMsg, CompositorReceiver, ConstellationMsg, SendableFrameTree, + CompositionPipeline, CompositorMsg, CompositorReceiver, ConstellationMsg, PaintMetricEvent, + SendableFrameTree, }; use crossbeam_channel::Sender; use dpi::PhysicalSize; @@ -33,9 +34,7 @@ use log::{debug, info, trace, warn}; use pixels::{CorsStatus, Image, ImageFrame, PixelFormat}; use profile_traits::time::{self as profile_time, ProfilerCategory}; use profile_traits::time_profile; -use script_traits::{ - AnimationState, AnimationTickType, ScriptThreadMessage, WindowSizeData, WindowSizeType, -}; +use script_traits::{AnimationState, AnimationTickType, WindowSizeData, WindowSizeType}; use servo_config::opts; use servo_geometry::DeviceIndependentPixel; use style_traits::{CSSPixel, PinchZoomFactor}; @@ -200,6 +199,21 @@ bitflags! { } } +/// The paint status of a particular pipeline in the Servo renderer. This is used to trigger metrics +/// in script (via the constellation) when display lists are received. +/// +/// See . +#[derive(PartialEq)] +pub(crate) enum PaintMetricState { + /// The renderer is still waiting to process a display list which triggers this metric. + Waiting, + /// The renderer has processed the display list which will trigger this event, marked the Servo + /// instance ready to paint, and is waiting for the given epoch to actually be rendered. + Seen(WebRenderEpoch, bool /* first_reflow */), + /// The metric has been sent to the constellation and no more work needs to be done. + Sent, +} + pub(crate) struct PipelineDetails { /// The pipeline associated with this PipelineDetails object. pub pipeline: Option, @@ -231,12 +245,11 @@ pub(crate) struct PipelineDetails { /// nodes in the compositor before forwarding new offsets to WebRender. pub scroll_tree: ScrollTree, - /// A per-pipeline queue of display lists that have not yet been rendered by WebRender. Layout - /// expects WebRender to paint each given epoch. Once the compositor paints a frame with that - /// epoch's display list, it will be removed from the queue and the paint time will be recorded - /// as a metric. In case new display lists come faster than painting a metric might never be - /// recorded. - pub pending_paint_metrics: Vec, + /// The paint metric status of the first paint. + pub first_paint_metric: PaintMetricState, + + /// The paint metric status of the first contentful paint. + pub first_contentful_paint_metric: PaintMetricState, } impl PipelineDetails { @@ -287,7 +300,8 @@ impl PipelineDetails { throttled: false, hit_test_items: Vec::new(), scroll_tree: ScrollTree::default(), - pending_paint_metrics: Vec::new(), + first_paint_metric: PaintMetricState::Waiting, + first_contentful_paint_metric: PaintMetricState::Waiting, } } @@ -648,12 +662,6 @@ impl IOCompositor { webview.dispatch_input_event(InputEvent::MouseMove(MouseMoveEvent { point })); }, - CompositorMsg::PendingPaintMetric(webview_id, pipeline_id, epoch) => { - if let Some(webview) = self.webviews.get_mut(webview_id) { - webview.add_pending_paint_metric(pipeline_id, epoch); - } - }, - CompositorMsg::CrossProcess(cross_proces_message) => { self.handle_cross_process_message(cross_proces_message); }, @@ -771,6 +779,18 @@ impl IOCompositor { details.hit_test_items = display_list_info.hit_test_info; details.install_new_scroll_tree(display_list_info.scroll_tree); + let epoch = display_list_info.epoch; + let first_reflow = display_list_info.first_reflow; + if details.first_paint_metric == PaintMetricState::Waiting { + details.first_paint_metric = PaintMetricState::Seen(epoch, first_reflow); + } + if details.first_contentful_paint_metric == PaintMetricState::Waiting && + display_list_info.is_contentful + { + details.first_contentful_paint_metric = + PaintMetricState::Seen(epoch, first_reflow); + } + let mut transaction = Transaction::new(); transaction .set_display_list(display_list_info.epoch, (pipeline_id, built_display_list)); @@ -1529,16 +1549,8 @@ impl IOCompositor { let paint_time = CrossProcessInstant::now(); let document_id = self.webrender_document(); for webview_details in self.webviews.iter_mut() { - // For each pipeline, determine the current epoch and update paint timing if necessary. for (pipeline_id, pipeline) in webview_details.pipelines.iter_mut() { - if pipeline.pending_paint_metrics.is_empty() { - continue; - } - let Some(composition_pipeline) = pipeline.pipeline.as_ref() else { - continue; - }; - - let Some(WebRenderEpoch(current_epoch)) = self + let Some(current_epoch) = self .webrender .as_ref() .and_then(|wr| wr.current_epoch(document_id, pipeline_id.into())) @@ -1546,29 +1558,43 @@ impl IOCompositor { continue; }; - let current_epoch = Epoch(current_epoch); - let Some(index) = pipeline - .pending_paint_metrics - .iter() - .position(|epoch| *epoch == current_epoch) - else { - continue; - }; + match pipeline.first_paint_metric { + // We need to check whether the current epoch is later, because + // CrossProcessCompositorMessage::SendInitialTransaction sends an + // empty display list to WebRender which can happen before we receive + // the first "real" display list. + PaintMetricState::Seen(epoch, first_reflow) if epoch <= current_epoch => { + assert!(epoch <= current_epoch); + if let Err(error) = self.global.borrow().constellation_sender.send( + ConstellationMsg::PaintMetric( + *pipeline_id, + PaintMetricEvent::FirstPaint(paint_time, first_reflow), + ), + ) { + warn!( + "Sending paint metric event to constellation failed ({error:?})." + ); + } + pipeline.first_paint_metric = PaintMetricState::Sent; + }, + _ => {}, + } - // Remove all epochs that were pending before the current epochs. They were not and will not, - // be painted. - pipeline.pending_paint_metrics.drain(0..index); - - if let Err(error) = - composition_pipeline - .script_chan - .send(ScriptThreadMessage::SetEpochPaintTime( - *pipeline_id, - current_epoch, - paint_time, - )) - { - warn!("Sending RequestLayoutPaintMetric message to layout failed ({error:?})."); + match pipeline.first_contentful_paint_metric { + PaintMetricState::Seen(epoch, first_reflow) if epoch <= current_epoch => { + if let Err(error) = self.global.borrow().constellation_sender.send( + ConstellationMsg::PaintMetric( + *pipeline_id, + PaintMetricEvent::FirstContentfulPaint(paint_time, first_reflow), + ), + ) { + warn!( + "Sending paint metric event to constellation failed ({error:?})." + ); + } + pipeline.first_contentful_paint_metric = PaintMetricState::Sent; + }, + _ => {}, } } } diff --git a/components/compositing/tracing.rs b/components/compositing/tracing.rs index 1ef0b41971c..907c931aebe 100644 --- a/components/compositing/tracing.rs +++ b/components/compositing/tracing.rs @@ -39,7 +39,6 @@ mod from_constellation { Self::SetThrottled(..) => target!("SetThrottled"), Self::NewWebRenderFrameReady(..) => target!("NewWebRenderFrameReady"), Self::PipelineExited(..) => target!("PipelineExited"), - Self::PendingPaintMetric(..) => target!("PendingPaintMetric"), Self::LoadComplete(..) => target!("LoadComplete"), Self::WebDriverMouseButtonEvent(..) => target!("WebDriverMouseButtonEvent"), Self::WebDriverMouseMoveEvent(..) => target!("WebDriverMouseMoveEvent"), diff --git a/components/compositing/webview.rs b/components/compositing/webview.rs index e912a9a8a7f..67aba73321f 100644 --- a/components/compositing/webview.rs +++ b/components/compositing/webview.rs @@ -252,12 +252,6 @@ impl WebView { } } - pub(crate) fn add_pending_paint_metric(&mut self, pipeline_id: PipelineId, epoch: base::Epoch) { - self.ensure_pipeline_details(pipeline_id) - .pending_paint_metrics - .push(epoch); - } - /// On a Window refresh tick (e.g. vsync) pub fn on_vsync(&mut self) { if let Some(fling_action) = self.touch_handler.on_vsync() { diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 6734eb6f46f..192e2485547 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -109,7 +109,8 @@ use canvas_traits::ConstellationCanvasMsg; use canvas_traits::canvas::{CanvasId, CanvasMsg}; use canvas_traits::webgl::WebGLThreads; use compositing_traits::{ - CompositorMsg, CompositorProxy, ConstellationMsg as FromCompositorMsg, SendableFrameTree, + CompositorMsg, CompositorProxy, ConstellationMsg as FromCompositorMsg, PaintMetricEvent, + SendableFrameTree, }; use crossbeam_channel::{Receiver, Sender, select, unbounded}; use devtools_traits::{ @@ -141,8 +142,8 @@ use script_traits::{ AnimationState, AnimationTickType, AuxiliaryWebViewCreationRequest, AuxiliaryWebViewCreationResponse, BroadcastMsg, ConstellationInputEvent, DiscardBrowsingContext, DocumentActivity, DocumentState, IFrameLoadInfo, - IFrameLoadInfoWithData, IFrameSandboxState, IFrameSizeMsg, Job, LayoutMsg as FromLayoutMsg, - LoadData, LoadOrigin, LogEntry, MessagePortMsg, NavigationHistoryBehavior, PortMessageTask, + IFrameLoadInfoWithData, IFrameSandboxState, IFrameSizeMsg, Job, LoadData, LoadOrigin, LogEntry, + MessagePortMsg, NavigationHistoryBehavior, PortMessageTask, ProgressiveWebMetricType, SWManagerMsg, SWManagerSenders, ScriptMsg as FromScriptMsg, ScriptThreadMessage, ScriptToConstellationChan, ServiceWorkerManagerFactory, ServiceWorkerMsg, StructuredSerializedData, UpdatePipelineIdReason, WindowSizeData, WindowSizeType, @@ -309,14 +310,6 @@ pub struct Constellation { /// dependency between script and layout. layout_factory: Arc, - /// An IPC channel for layout to send messages to the constellation. - /// This is the layout's view of `layout_receiver`. - layout_sender: IpcSender, - - /// A channel for the constellation to receive messages from layout. - /// This is the constellation's view of `layout_sender`. - layout_receiver: Receiver>, - /// A channel for the constellation to receive messages from the compositor thread. compositor_receiver: Receiver, @@ -661,13 +654,6 @@ where ) }; - let (layout_ipc_sender, layout_ipc_receiver) = - ipc::channel().expect("ipc channel failure"); - let layout_receiver = - route_ipc_receiver_to_new_crossbeam_receiver_preserving_errors( - layout_ipc_receiver, - ); - let swmanager_receiver = route_ipc_receiver_to_new_crossbeam_receiver_preserving_errors( swmanager_ipc_receiver, @@ -693,11 +679,9 @@ where background_hang_monitor_receiver, background_monitor_register, background_monitor_control_senders: background_hang_monitor_control_ipc_senders, - layout_sender: layout_ipc_sender, script_receiver, compositor_receiver, layout_factory, - layout_receiver, embedder_proxy: state.embedder_proxy, compositor_proxy: state.compositor_proxy, webviews: WebViewManager::default(), @@ -967,7 +951,6 @@ where background_hang_monitor_to_constellation_chan: self .background_hang_monitor_sender .clone(), - layout_to_constellation_chan: self.layout_sender.clone(), layout_factory: self.layout_factory.clone(), compositor_proxy: self.compositor_proxy.clone(), devtools_sender: self.devtools_sender.clone(), @@ -1132,7 +1115,6 @@ where Script((PipelineId, FromScriptMsg)), BackgroundHangMonitor(HangMonitorAlert), Compositor(FromCompositorMsg), - Layout(FromLayoutMsg), FromSWManager(SWManagerMsg), } // Get one incoming request. @@ -1163,9 +1145,6 @@ where recv(self.compositor_receiver) -> msg => { Ok(Request::Compositor(msg.expect("Unexpected compositor channel panic in constellation"))) } - recv(self.layout_receiver) -> msg => { - msg.expect("Unexpected layout channel panic in constellation").map(Request::Layout) - } recv(self.swmanager_receiver) -> msg => { msg.expect("Unexpected SW channel panic in constellation").map(Request::FromSWManager) } @@ -1188,9 +1167,6 @@ where Request::BackgroundHangMonitor(message) => { self.handle_request_from_background_hang_monitor(message); }, - Request::Layout(message) => { - self.handle_request_from_layout(message); - }, Request::FromSWManager(message) => { self.handle_request_from_swmanager(message); }, @@ -1409,6 +1385,9 @@ where FromCompositorMsg::SetScrollStates(pipeline_id, scroll_states) => { self.handle_set_scroll_states(pipeline_id, scroll_states) }, + FromCompositorMsg::PaintMetric(pipeline_id, paint_metric_event) => { + self.handle_paint_metric(pipeline_id, paint_metric_event); + }, } } @@ -1980,19 +1959,6 @@ where } } - #[cfg_attr( - feature = "tracing", - tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") - )] - fn handle_request_from_layout(&mut self, message: FromLayoutMsg) { - trace_layout_msg!(message, "{message:?}"); - match message { - FromLayoutMsg::PendingPaintMetric(webview_id, pipeline_id, epoch) => { - self.handle_pending_paint_metric(webview_id, pipeline_id, epoch); - }, - } - } - #[cfg_attr( feature = "tracing", tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") @@ -3401,24 +3367,6 @@ where }); } - #[cfg_attr( - feature = "tracing", - tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") - )] - fn handle_pending_paint_metric( - &self, - webview_id: WebViewId, - pipeline_id: PipelineId, - epoch: Epoch, - ) { - self.compositor_proxy - .send(CompositorMsg::PendingPaintMetric( - webview_id, - pipeline_id, - epoch, - )) - } - #[cfg_attr( feature = "tracing", tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") @@ -5585,4 +5533,35 @@ where warn!("Could not send scroll offsets to pipeline: {pipeline_id:?}: {error:?}"); } } + + #[cfg_attr( + feature = "tracing", + tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") + )] + fn handle_paint_metric(&mut self, pipeline_id: PipelineId, event: PaintMetricEvent) { + let Some(pipeline) = self.pipelines.get(&pipeline_id) else { + warn!("Discarding paint metric event for unknown pipeline"); + return; + }; + let (metric_type, metric_value, first_reflow) = match event { + PaintMetricEvent::FirstPaint(metric_value, first_reflow) => ( + ProgressiveWebMetricType::FirstPaint, + metric_value, + first_reflow, + ), + PaintMetricEvent::FirstContentfulPaint(metric_value, first_reflow) => ( + ProgressiveWebMetricType::FirstContentfulPaint, + metric_value, + first_reflow, + ), + }; + if let Err(error) = pipeline.event_loop.send(ScriptThreadMessage::PaintMetric( + pipeline_id, + metric_type, + metric_value, + first_reflow, + )) { + warn!("Could not sent paint metric event to pipeline: {pipeline_id:?}: {error:?}"); + } + } } diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 39b3fce26df..f394c90b6c2 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -34,9 +34,8 @@ use net_traits::image_cache::ImageCache; use profile_traits::{mem as profile_mem, time}; use script_layout_interface::{LayoutFactory, ScriptThreadFactory}; use script_traits::{ - AnimationState, DiscardBrowsingContext, DocumentActivity, InitialScriptState, LayoutMsg, - LoadData, NewLayoutInfo, SWManagerMsg, ScriptThreadMessage, ScriptToConstellationChan, - WindowSizeData, + AnimationState, DiscardBrowsingContext, DocumentActivity, InitialScriptState, LoadData, + NewLayoutInfo, SWManagerMsg, ScriptThreadMessage, ScriptToConstellationChan, WindowSizeData, }; use serde::{Deserialize, Serialize}; use servo_config::opts::{self, Opts}; @@ -133,9 +132,6 @@ pub struct InitialPipelineState { /// A channel for the background hang monitor to send messages to the constellation. pub background_hang_monitor_to_constellation_chan: IpcSender, - /// A channel for the layout to send messages to the constellation. - pub layout_to_constellation_chan: IpcSender, - /// A fatory for creating layouts to be used by the ScriptThread. pub layout_factory: Arc, @@ -279,7 +275,6 @@ impl Pipeline { time_profiler_chan: state.time_profiler_chan, mem_profiler_chan: state.mem_profiler_chan, window_size: state.window_size, - layout_to_constellation_chan: state.layout_to_constellation_chan, script_chan: script_chan.clone(), load_data: state.load_data.clone(), script_port, @@ -481,7 +476,6 @@ pub struct UnprivilegedPipelineContent { script_to_constellation_chan: ScriptToConstellationChan, background_hang_monitor_to_constellation_chan: IpcSender, bhm_control_port: Option>, - layout_to_constellation_chan: IpcSender, devtools_ipc_sender: Option>, #[cfg(feature = "bluetooth")] bluetooth_thread: IpcSender, @@ -533,7 +527,6 @@ impl UnprivilegedPipelineContent { constellation_receiver: self.script_port, pipeline_to_constellation_sender: self.script_to_constellation_chan.clone(), background_hang_monitor_register: background_hang_monitor_register.clone(), - layout_to_constellation_ipc_sender: self.layout_to_constellation_chan.clone(), #[cfg(feature = "bluetooth")] bluetooth_sender: self.bluetooth_thread, resource_threads: self.resource_threads, diff --git a/components/constellation/tracing.rs b/components/constellation/tracing.rs index 38dda83538f..efd3c0e9566 100644 --- a/components/constellation/tracing.rs +++ b/components/constellation/tracing.rs @@ -30,17 +30,6 @@ macro_rules! trace_script_msg { }; } -/// Log an event from layout at trace level. -/// - To disable tracing: RUST_LOG='constellation { - ::log::trace!(target: $crate::tracing::LogTarget::log_target(&$event), $($rest)+) - }; -} - /// Get the log target for an event, as a static string. pub(crate) trait LogTarget { fn log_target(&self) -> &'static str; @@ -87,6 +76,7 @@ mod from_compositor { Self::MediaSessionAction(_) => target!("MediaSessionAction"), Self::SetWebViewThrottled(_, _) => target!("SetWebViewThrottled"), Self::SetScrollStates(..) => target!("SetScrollStates"), + Self::PaintMetric(..) => target!("PaintMetric"), } } } @@ -249,21 +239,3 @@ mod from_script { } } } - -mod from_layout { - use super::LogTarget; - - macro_rules! target { - ($($name:literal)+) => { - concat!("constellation &'static str { - match self { - Self::PendingPaintMetric(..) => target!("PendingPaintMetric"), - } - } - } -} diff --git a/components/layout_2020/display_list/mod.rs b/components/layout_2020/display_list/mod.rs index 696aecefeda..4ba62041099 100644 --- a/components/layout_2020/display_list/mod.rs +++ b/components/layout_2020/display_list/mod.rs @@ -102,6 +102,7 @@ impl DisplayList { pipeline_id: wr::PipelineId, epoch: wr::Epoch, viewport_scroll_sensitivity: AxesScrollSensitivity, + first_reflow: bool, ) -> Self { Self { wr: wr::DisplayListBuilder::new(pipeline_id), @@ -111,6 +112,7 @@ impl DisplayList { pipeline_id, epoch, viewport_scroll_sensitivity, + first_reflow, ), spatial_tree_count: 0, } @@ -161,11 +163,6 @@ pub(crate) struct DisplayListBuilder<'a> { /// The [DisplayList] used to collect display list items and metadata. pub display_list: &'a mut DisplayList, - - /// Contentful paint i.e. whether the display list contains items of type - /// text, image, non-white canvas or SVG). Used by metrics. - /// See . - is_contentful: bool, } impl DisplayList { @@ -175,7 +172,7 @@ impl DisplayList { context: &LayoutContext, fragment_tree: &FragmentTree, root_stacking_context: &StackingContext, - ) -> bool { + ) { #[cfg(feature = "tracing")] let _span = tracing::trace_span!("display_list::build", servo_profiling = true).entered(); let mut builder = DisplayListBuilder { @@ -183,12 +180,10 @@ impl DisplayList { current_reference_frame_scroll_node_id: self.compositor_info.root_reference_frame_id, current_clip_chain_id: ClipChainId::INVALID, element_for_canvas_background: fragment_tree.canvas_background.from_element, - is_contentful: false, context, display_list: self, }; fragment_tree.build_display_list(&mut builder, root_stacking_context); - builder.is_contentful } } @@ -197,6 +192,10 @@ impl DisplayListBuilder<'_> { &mut self.display_list.wr } + fn mark_is_contentful(&mut self) { + self.display_list.compositor_info.is_contentful = true; + } + fn common_properties( &self, clip_rect: units::LayoutRect, @@ -282,7 +281,7 @@ impl Fragment { let image = image.borrow(); match image.style.get_inherited_box().visibility { Visibility::Visible => { - builder.is_contentful = true; + builder.mark_is_contentful(); let image_rendering = image .style @@ -318,7 +317,7 @@ impl Fragment { let iframe = iframe.borrow(); match iframe.style.get_inherited_box().visibility { Visibility::Visible => { - builder.is_contentful = true; + builder.mark_is_contentful(); let rect = iframe.rect.translate(containing_block.origin.to_vector()); let common = builder.common_properties(rect.to_webrender(), &iframe.style); @@ -384,7 +383,7 @@ impl Fragment { // NB: The order of painting text components (CSS Text Decoration Module Level 3) is: // shadows, underline, overline, text, text-emphasis, and then line-through. - builder.is_contentful = true; + builder.mark_is_contentful(); let rect = fragment.rect.translate(containing_block.origin.to_vector()); let mut baseline_origin = rect.origin; diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index a0e7de3081f..38ce62d833a 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -16,7 +16,6 @@ use std::sync::{Arc, LazyLock}; use app_units::Au; use base::Epoch; -use base::cross_process_instant::CrossProcessInstant; use base::id::{PipelineId, WebViewId}; use embedder_traits::resources::{self, Resource}; use euclid::default::{Point2D as UntypedPoint2D, Rect as UntypedRect, Size2D as UntypedSize2D}; @@ -37,7 +36,6 @@ use layout::traversal::RecalcStyle; use layout::{BoxTree, FragmentTree}; use log::{debug, error}; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; -use metrics::{PaintTimeMetrics, ProfilerMetadataFactory}; use net_traits::image_cache::{ImageCache, UsePlaceholder}; use parking_lot::{Mutex, RwLock}; use profile_traits::mem::{Report, ReportKind}; @@ -160,9 +158,6 @@ pub struct LayoutThread { /// Cross-process access to the Compositor API. compositor_api: CrossProcessCompositorApi, - /// Paint time metrics. - paint_time_metrics: PaintTimeMetrics, - /// Debug options, copied from configuration to this `LayoutThread` in order /// to avoid having to constantly access the thread-safe global options. debug: DebugOptions, @@ -462,10 +457,6 @@ impl Layout for LayoutThread { .map(|scroll_state| (scroll_state.scroll_id, scroll_state.scroll_offset)) .collect(); } - - fn set_epoch_paint_time(&mut self, epoch: Epoch, paint_time: CrossProcessInstant) { - self.paint_time_metrics.maybe_set_metric(epoch, paint_time); - } } impl LayoutThread { @@ -520,7 +511,6 @@ impl LayoutThread { scroll_offsets: Default::default(), stylist: Stylist::new(device, QuirksMode::NoQuirks), webrender_image_cache: Default::default(), - paint_time_metrics: config.paint_time_metrics, debug: opts::get().debug.clone(), } } @@ -856,6 +846,7 @@ impl LayoutThread { self.id.into(), epoch.into(), fragment_tree.viewport_scroll_sensitivity, + self.first_reflow.get(), ); display_list.wr.begin(); @@ -874,7 +865,7 @@ impl LayoutThread { display_list.build_stacking_context_tree(&fragment_tree, &self.debug); // Build the rest of the display list which inclues all of the WebRender primitives. - let is_contentful = display_list.build(context, &fragment_tree, &root_stacking_context); + display_list.build(context, &fragment_tree, &root_stacking_context); if self.debug.dump_flow_tree { fragment_tree.print(); @@ -884,12 +875,6 @@ impl LayoutThread { } debug!("Layout done!"); - // Observe notifications about rendered frames if needed right before - // sending the display list to WebRender in order to set time related - // Progressive Web Metrics. - self.paint_time_metrics - .maybe_observe_paint_time(self, epoch, is_contentful); - if reflow_goal.needs_display() { self.compositor_api.send_display_list( self.webview_id, @@ -991,12 +976,6 @@ impl LayoutThread { } } -impl ProfilerMetadataFactory for LayoutThread { - fn new_metadata(&self) -> Option { - self.profiler_metadata() - } -} - fn get_ua_stylesheets() -> Result { fn parse_ua_stylesheet( shared_lock: &SharedRwLock, diff --git a/components/metrics/lib.rs b/components/metrics/lib.rs index d8237fe2deb..4f42c2f7bbe 100644 --- a/components/metrics/lib.rs +++ b/components/metrics/lib.rs @@ -2,38 +2,20 @@ * 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, RefCell}; +use std::cell::Cell; use std::cmp::Ordering; -use std::collections::HashMap; use std::time::Duration; -use base::Epoch; use base::cross_process_instant::CrossProcessInstant; -use base::id::{PipelineId, WebViewId}; -use ipc_channel::ipc::IpcSender; -use log::warn; use malloc_size_of_derive::MallocSizeOf; -use profile_traits::time::{ProfilerCategory, ProfilerChan, TimerMetadata, send_profile_data}; -use script_traits::{LayoutMsg, ProgressiveWebMetricType, ScriptThreadMessage}; +use profile_traits::time::{ + ProfilerCategory, ProfilerChan, TimerMetadata, TimerMetadataFrameType, TimerMetadataReflowType, + send_profile_data, +}; +use script_traits::ProgressiveWebMetricType; use servo_config::opts; use servo_url::ServoUrl; -pub trait ProfilerMetadataFactory { - fn new_metadata(&self) -> Option; -} - -pub trait ProgressiveWebMetric { - fn get_navigation_start(&self) -> Option; - fn set_navigation_start(&mut self, time: CrossProcessInstant); - fn get_time_profiler_chan(&self) -> &ProfilerChan; - fn send_queued_constellation_msg( - &self, - name: ProgressiveWebMetricType, - time: CrossProcessInstant, - ); - fn get_url(&self) -> &ServoUrl; -} - /// TODO make this configurable /// maximum task time is 50ms (in ns) pub const MAX_TASK_NS: u64 = 50000000; @@ -50,8 +32,8 @@ impl ToMs for u64 { } } -fn set_metric( - pwm: &U, +fn set_metric( + pwm: &ProgressiveWebMetrics, metadata: Option, metric_type: ProgressiveWebMetricType, category: ProfilerCategory, @@ -61,14 +43,11 @@ fn set_metric( ) { attr.set(Some(metric_time)); - // Queue performance observer notification. - pwm.send_queued_constellation_msg(metric_type, metric_time); - // Send the metric to the time profiler. send_profile_data( category, metadata, - pwm.get_time_profiler_chan(), + pwm.time_profiler_chan(), metric_time, metric_time, ); @@ -76,7 +55,7 @@ fn set_metric( // Print the metric to console if the print-pwm option was given. if opts::get().print_pwm { let navigation_start = pwm - .get_navigation_start() + .navigation_start() .unwrap_or_else(CrossProcessInstant::epoch); println!( "{:?} {:?} {:?}", @@ -87,14 +66,19 @@ fn set_metric( } } -// spec: https://github.com/WICG/time-to-interactive -// https://github.com/GoogleChrome/lighthouse/issues/27 -// we can look at three different metrics here: -// navigation start -> visually ready (dom content loaded) -// navigation start -> thread ready (main thread available) -// visually ready -> thread ready +/// A data structure to track web metrics dfined in various specifications: +/// +/// - +/// - / +/// +/// We can look at three different metrics here: +/// - navigation start -> visually ready (dom content loaded) +/// - navigation start -> thread ready (main thread available) +/// - visually ready -> thread ready #[derive(MallocSizeOf)] -pub struct InteractiveMetrics { +pub struct ProgressiveWebMetrics { + /// Whether or not this metric is for an `