Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

measure blocked layout queries #23115

Merged
merged 2 commits into from Apr 22, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -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 {
@@ -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,
self.busy.clone(),

This comment has been minimized.

Copy link
@jdm

jdm Mar 29, 2019

Member

We should be passing an AtomicBoolean that is stored in LayoutThreadInit, rather than cloning the one that is used by the current layout thread.

This comment has been minimized.

Copy link
@jdm

jdm Apr 16, 2019

Member

My previous comment here is still valid - there's now a member of LayoutThreadInit that we should be passing here instead.

);
}

@@ -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>,
);
}
@@ -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();

This comment has been minimized.

Copy link
@jdm

jdm Apr 16, 2019

Member

Can you paste some example output that you saw?

}
writeln!(&mut lock, "").unwrap();
},
Some(OutputOptions::DB(ref hostname, ref dbname, ref user, ref password)) => {
// Unfortunately, influent does not like hostnames ending with "/"
@@ -46,6 +46,11 @@ pub enum ProfilerMsg {
),
/// Message used to force print the profiling metrics
Print,

// TODO pylbrecht
// write meaningful docstring

This comment has been minimized.

Copy link
@jdm

jdm Apr 16, 2019

Member

/// Report a layout query that could not be processed immediately for a particular URL.

BlockedLayoutQuery(String),

/// Tells the profiler to shut down.
Exit(IpcSender<()>),
}
@@ -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) }
@@ -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 to indicate if the layout thread is busy handling a request.

This comment has been minimized.

Copy link
@jdm

jdm Apr 16, 2019

Member

/// 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);

@@ -2025,6 +2032,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(),
@@ -2043,6 +2052,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
@@ -2076,6 +2086,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);
@@ -2872,6 +2883,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.
@@ -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>,
}
@@ -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`
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.