Auto merge of #23115 - pylbrecht:measure.blocked.layout.queries, r=jdm

measure blocked layout queries

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #19797

<!-- Either: -->
- [ ] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23115)
<!-- Reviewable:end -->

components/constellation/pipeline.rs

@@ -40,6 +40,7 @@ use std::env;
 use std::ffi::OsStr;
 use std::process;
 use std::rc::Rc;
+use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
 use style_traits::CSSPixel;
 use style_traits::DevicePixel;
@@ -528,6 +529,7 @@ impl UnprivilegedPipelineContent {
             self.script_chan.clone(),
             self.load_data.url.clone(),
         );
+        let layout_thread_busy_flag = Arc::new(AtomicBool::new(false));
         let layout_pair = STF::create(
             InitialScriptState {
                 id: self.id,
@@ -554,6 +556,7 @@ impl UnprivilegedPipelineContent {
                 webvr_chan: self.webvr_chan,
                 webrender_document: self.webrender_document,
                 webrender_api_sender: self.webrender_api_sender.clone(),
+                layout_is_busy: layout_thread_busy_flag.clone(),
             },
             self.load_data.clone(),
         );
@@ -576,6 +579,7 @@ impl UnprivilegedPipelineContent {
             self.webrender_api_sender,
             self.webrender_document,
             paint_time_metrics,
+            layout_thread_busy_flag.clone(),
         );
 
         if wait_for_completion {

components/layout_thread/lib.rs

@@ -96,7 +96,7 @@ use std::cell::{Cell, RefCell};
 use std::collections::HashMap;
 use std::ops::{Deref, DerefMut};
 use std::process;
-use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex, MutexGuard};
 use std::thread;
 use std::time::Duration;
@@ -244,6 +244,9 @@ pub struct LayoutThread {
 
     /// The sizes of all iframes encountered during the last layout operation.
     last_iframe_sizes: RefCell<HashMap<BrowsingContextId, TypedSize2D<f32, CSSPixel>>>,
+
+    /// Flag that indicates if LayoutThread is busy handling a request.
+    busy: Arc<AtomicBool>,
 }
 
 impl LayoutThreadFactory for LayoutThread {
@@ -268,6 +271,7 @@ impl LayoutThreadFactory for LayoutThread {
         webrender_api_sender: webrender_api::RenderApiSender,
         webrender_document: webrender_api::DocumentId,
         paint_time_metrics: PaintTimeMetrics,
+        busy: Arc<AtomicBool>,
     ) {
         thread::Builder::new()
             .name(format!("LayoutThread {:?}", id))
@@ -305,6 +309,7 @@ impl LayoutThreadFactory for LayoutThread {
                         webrender_api_sender,
                         webrender_document,
                         paint_time_metrics,
+                        busy,
                     );
 
                     let reporter_name = format!("layout-reporter-{}", id);
@@ -466,6 +471,7 @@ impl LayoutThread {
         webrender_api_sender: webrender_api::RenderApiSender,
         webrender_document: webrender_api::DocumentId,
         paint_time_metrics: PaintTimeMetrics,
+        busy: Arc<AtomicBool>,
     ) -> LayoutThread {
         // The device pixel ratio is incorrect (it does not have the hidpi value),
         // but it will be set correctly when the initial reflow takes place.
@@ -544,6 +550,7 @@ impl LayoutThread {
             paint_time_metrics: paint_time_metrics,
             layout_query_waiting_time: Histogram::new(),
             last_iframe_sizes: Default::default(),
+            busy: busy,
         }
     }
 
@@ -648,7 +655,8 @@ impl LayoutThread {
             recv(self.font_cache_receiver) -> msg => { msg.unwrap(); Request::FromFontCache }
         };
 
-        match request {
+        self.busy.store(true, Ordering::Relaxed);
+        let result = match request {
             Request::FromPipeline(LayoutControlMsg::SetScrollStates(new_scroll_states)) => self
                 .handle_request_helper(
                     Msg::SetScrollStates(new_scroll_states),
@@ -679,7 +687,9 @@ impl LayoutThread {
                     .unwrap();
                 true
             },
-        }
+        };
+        self.busy.store(false, Ordering::Relaxed);
+        result
     }
 
     /// Receives and dispatches messages from other threads.
@@ -861,6 +871,7 @@ impl LayoutThread {
             self.webrender_api.clone_sender(),
             self.webrender_document,
             info.paint_time_metrics,
+            info.layout_is_busy,
         );
     }
 

components/layout_traits/lib.rs

@@ -20,6 +20,7 @@ use profile_traits::{mem, time};
 use script_traits::LayoutMsg as ConstellationMsg;
 use script_traits::{ConstellationControlMsg, LayoutControlMsg};
 use servo_url::ServoUrl;
+use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
 
 // A static method creating a layout thread
@@ -44,5 +45,6 @@ pub trait LayoutThreadFactory {
         webrender_api_sender: webrender_api::RenderApiSender,
         webrender_document: webrender_api::DocumentId,
         paint_time_metrics: PaintTimeMetrics,
+        busy: Arc<AtomicBool>,
     );
 }

components/profile/time.rs

@@ -18,7 +18,7 @@ use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType};
 use servo_config::opts::OutputOptions;
 use std::borrow::ToOwned;
 use std::cmp::Ordering;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap};
 use std::error::Error;
 use std::fs::File;
 use std::io::{self, Write};
@@ -171,6 +171,7 @@ pub struct Profiler {
     output: Option<OutputOptions>,
     pub last_msg: Option<ProfilerMsg>,
     trace: Option<TraceDump>,
+    blocked_layout_queries: HashMap<String, u32>,
 }
 
 impl Profiler {
@@ -305,6 +306,7 @@ impl Profiler {
             output: output,
             last_msg: None,
             trace: trace,
+            blocked_layout_queries: HashMap::new(),
         }
     }
 
@@ -345,6 +347,9 @@ impl Profiler {
                     None => sender.send(ProfilerData::NoRecords).unwrap(),
                 };
             },
+            ProfilerMsg::BlockedLayoutQuery(url) => {
+                *self.blocked_layout_queries.entry(url).or_insert(0) += 1;
+            },
             ProfilerMsg::Exit(chan) => {
                 heartbeats::cleanup();
                 self.print_buckets();
@@ -411,6 +416,11 @@ impl Profiler {
                         .unwrap();
                     }
                 }
+
+                write!(file, "_url\t_blocked layout queries_\n").unwrap();
+                for (url, count) in &self.blocked_layout_queries {
+                    write!(file, "{}\t{}\n", url, count).unwrap();
+                }
             },
             Some(OutputOptions::Stdout(_)) => {
                 let stdout = io::stdout();
@@ -450,6 +460,12 @@ impl Profiler {
                     }
                 }
                 writeln!(&mut lock, "").unwrap();
+
+                writeln!(&mut lock, "_url_\t_blocked layout queries_").unwrap();
+                for (url, count) in &self.blocked_layout_queries {
+                    writeln!(&mut lock, "{}\t{}", url, count).unwrap();
+                }
+                writeln!(&mut lock, "").unwrap();
             },
             Some(OutputOptions::DB(ref hostname, ref dbname, ref user, ref password)) => {
                 // Unfortunately, influent does not like hostnames ending with "/"

components/profile_traits/time.rs

@@ -46,6 +46,10 @@ pub enum ProfilerMsg {
     ),
     /// Message used to force print the profiling metrics
     Print,
+
+    /// Report a layout query that could not be processed immediately for a particular URL.
+    BlockedLayoutQuery(String),
+
     /// Tells the profiler to shut down.
     Exit(IpcSender<()>),
 }

components/script/dom/window.rs

@@ -92,7 +92,7 @@ use net_traits::{ReferrerPolicy, ResourceThreads};
 use num_traits::ToPrimitive;
 use profile_traits::ipc as ProfiledIpc;
 use profile_traits::mem::ProfilerChan as MemProfilerChan;
-use profile_traits::time::ProfilerChan as TimeProfilerChan;
+use profile_traits::time::{ProfilerChan as TimeProfilerChan, ProfilerMsg};
 use script_layout_interface::message::{Msg, QueryMsg, Reflow, ReflowGoal, ScriptReflow};
 use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC};
 use script_layout_interface::rpc::{
@@ -117,7 +117,7 @@ use std::fs;
 use std::io::{stderr, stdout, Write};
 use std::mem;
 use std::rc::Rc;
-use std::sync::atomic::Ordering;
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 use style::dom::OpaqueNode;
 use style::error_reporting::{ContextualParseError, ParseErrorReporter};
@@ -293,6 +293,10 @@ pub struct Window {
     /// Indicate whether a SetDocumentStatus message has been sent after a reflow is complete.
     /// It is used to avoid sending idle message more than once, which is unneccessary.
     has_sent_idle_message: Cell<bool>,
+
+    /// Flag that indicates if the layout thread is busy handling a request.
+    #[ignore_malloc_size_of = "Arc<T> is hard"]
+    layout_is_busy: Arc<AtomicBool>,
 }
 
 impl Window {
@@ -1558,6 +1562,12 @@ impl Window {
     }
 
     pub fn layout_reflow(&self, query_msg: QueryMsg) -> bool {
+        if self.layout_is_busy.load(Ordering::Relaxed) {
+            let url = self.get_url().into_string();
+            self.time_profiler_chan()
+                .send(ProfilerMsg::BlockedLayoutQuery(url));
+        }
+
         self.reflow(
             ReflowGoal::LayoutQuery(query_msg, time::precise_time_ns()),
             ReflowReason::Query,
@@ -2023,6 +2033,7 @@ impl Window {
         microtask_queue: Rc<MicrotaskQueue>,
         webrender_document: DocumentId,
         webrender_api_sender: RenderApiSender,
+        layout_is_busy: Arc<AtomicBool>,
     ) -> DomRoot<Self> {
         let layout_rpc: Box<dyn LayoutRPC + Send> = {
             let (rpc_send, rpc_recv) = unbounded();
@@ -2095,6 +2106,7 @@ impl Window {
             exists_mut_observer: Cell::new(false),
             webrender_api_sender,
             has_sent_idle_message: Cell::new(false),
+            layout_is_busy,
         });
 
         unsafe { WindowBinding::Wrap(runtime.cx(), win) }

components/script/script_thread.rs

@@ -144,6 +144,7 @@ use std::option::Option;
 use std::ptr;
 use std::rc::Rc;
 use std::result::Result;
+use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
 use std::thread;
 use std::time::{Duration, SystemTime};
@@ -202,6 +203,8 @@ struct InProgressLoad {
     navigation_start_precise: u64,
     /// For cancelling the fetch
     canceller: FetchCanceller,
+    /// Flag for sharing with the layout thread that is not yet created.
+    layout_is_busy: Arc<AtomicBool>,
 }
 
 impl InProgressLoad {
@@ -216,6 +219,7 @@ impl InProgressLoad {
         window_size: WindowSizeData,
         url: ServoUrl,
         origin: MutableOrigin,
+        layout_is_busy: Arc<AtomicBool>,
     ) -> InProgressLoad {
         let current_time = get_time();
         let navigation_start_precise = precise_time_ns();
@@ -237,6 +241,7 @@ impl InProgressLoad {
             navigation_start: (current_time.sec * 1000 + current_time.nsec as i64 / 1000000) as u64,
             navigation_start_precise: navigation_start_precise,
             canceller: Default::default(),
+            layout_is_busy: layout_is_busy,
         }
     }
 }
@@ -702,6 +707,7 @@ impl ScriptThreadFactory for ScriptThread {
                 let opener = state.opener;
                 let mem_profiler_chan = state.mem_profiler_chan.clone();
                 let window_size = state.window_size;
+                let layout_is_busy = state.layout_is_busy.clone();
 
                 let script_thread = ScriptThread::new(state, script_port, script_chan.clone());
 
@@ -722,6 +728,7 @@ impl ScriptThreadFactory for ScriptThread {
                     window_size,
                     load_data.url.clone(),
                     origin,
+                    layout_is_busy,
                 );
                 script_thread.pre_page_load(new_load, load_data);
 
@@ -2028,6 +2035,8 @@ impl ScriptThread {
         let layout_pair = unbounded();
         let layout_chan = layout_pair.0.clone();
 
+        let layout_is_busy = Arc::new(AtomicBool::new(false));
+
         let msg = message::Msg::CreateLayoutThread(LayoutThreadInit {
             id: new_pipeline_id,
             url: load_data.url.clone(),
@@ -2046,6 +2055,7 @@ impl ScriptThread {
                 self.control_chan.clone(),
                 load_data.url.clone(),
             ),
+            layout_is_busy: layout_is_busy.clone(),
         });
 
         // Pick a layout thread, any layout thread
@@ -2079,6 +2089,7 @@ impl ScriptThread {
             window_size,
             load_data.url.clone(),
             origin,
+            layout_is_busy.clone(),
         );
         if load_data.url.as_str() == "about:blank" {
             self.start_page_load_about_blank(new_load, load_data.js_eval_result);
@@ -2875,6 +2886,7 @@ impl ScriptThread {
             self.microtask_queue.clone(),
             self.webrender_document,
             self.webrender_api_sender.clone(),
+            incomplete.layout_is_busy,
         );
 
         // Initialize the browsing context for the window.

components/script_layout_interface/message.rs

@@ -19,6 +19,7 @@ use script_traits::{ScrollState, UntrustedNodeAddress, WindowSizeData};
 use servo_arc::Arc as ServoArc;
 use servo_atoms::Atom;
 use servo_url::ServoUrl;
+use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
 use style::context::QuirksMode;
 use style::dom::OpaqueNode;
@@ -226,4 +227,5 @@ pub struct LayoutThreadInit {
     pub image_cache: Arc<dyn ImageCache>,
     pub content_process_shutdown_chan: Option<IpcSender<()>>,
     pub paint_time_metrics: PaintTimeMetrics,
+    pub layout_is_busy: Arc<AtomicBool>,
 }

components/script_traits/lib.rs

@@ -50,6 +50,7 @@ use servo_url::ImmutableOrigin;
 use servo_url::ServoUrl;
 use std::collections::HashMap;
 use std::fmt;
+use std::sync::atomic::AtomicBool;
 use std::sync::Arc;
 use std::time::Duration;
 use style_traits::CSSPixel;
@@ -600,6 +601,8 @@ pub struct InitialScriptState {
     pub webrender_document: DocumentId,
     /// FIXME(victor): The Webrender API sender in this constellation's pipeline
     pub webrender_api_sender: RenderApiSender,
+    /// Flag to indicate if the layout thread is busy handling a request.
+    pub layout_is_busy: Arc<AtomicBool>,
 }
 
 /// This trait allows creating a `ScriptThread` without depending on the `script`