servo: Merge #19218 - added navigation start for interactive metrics (from avadacatavra:nav-start); r=jdm
authorddh <dianehosfelt@gmail.com>
Tue, 14 Nov 2017 09:53:30 -0600
changeset 443512 a3be64eeb28c9284dff61b1a467e9c6eae4546a7
parent 443511 f872d28315f03697c0742db23a8ad8cc8484900b
child 443513 bbe53fb92e21bfd5ec6ace0a854a483dbf4514ab
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #19218 - added navigation start for interactive metrics (from avadacatavra:nav-start); r=jdm <!-- Please describe your changes on the following line: --> follow up from #18670, fixing #19099 --- <!-- 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 #19099 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- 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. --> Source-Repo: https://github.com/servo/servo Source-Revision: 7d13e5b83c346d06dc68f22857a17f30ea797402
servo/components/metrics/lib.rs
servo/components/script/dom/document.rs
servo/components/script/script_thread.rs
--- a/servo/components/metrics/lib.rs
+++ b/servo/components/metrics/lib.rs
@@ -35,16 +35,17 @@ pub trait ProfilerMetadataFactory {
 
 pub trait ProgressiveWebMetric {
     fn get_navigation_start(&self) -> Option<u64>;
     fn set_navigation_start(&mut self, time: u64);
     fn get_time_profiler_chan(&self) -> &ProfilerChan;
     fn send_queued_constellation_msg(&self, name: ProgressiveWebMetricType, time: u64);
 }
 
+/// TODO make this configurable
 /// maximum task time is 50ms (in ns)
 pub const MAX_TASK_NS: u64 = 50000000;
 /// 10 second window (in ns)
 const INTERACTIVE_WINDOW_SECONDS_IN_NS: u64 = 10000000000;
 
 
 fn set_metric<U: ProgressiveWebMetric>(
     pwm: &U,
@@ -79,16 +80,17 @@ fn set_metric<U: ProgressiveWebMetric>(
         time,
         time,
         0,
         0,
     );
 
     // Print the metric to console if the print-pwm option was given.
     if opts::get().print_pwm {
+        println!("Navigation start: {}", pwm.get_navigation_start().unwrap());
         println!("{:?} {:?}", metric_type, time);
     }
 
 }
 
 // https://github.com/GoogleChrome/lighthouse/issues/27
 // we can look at three different metrics here:
 // navigation start -> visually ready (dom content loaded)
@@ -211,16 +213,20 @@ impl InteractiveMetrics {
             ProfilerCategory::TimeToInteractive,
             &self.time_to_interactive,
             Some(metric_time));
     }
 
     pub fn get_tti(&self) -> Option<u64> {
         self.time_to_interactive.get()
     }
+
+    pub fn needs_tti(&self) -> bool {
+        self.get_tti().is_none()
+    }
 }
 
 impl ProgressiveWebMetric for InteractiveMetrics {
     fn get_navigation_start(&self) -> Option<u64> {
         self.navigation_start
     }
 
     fn set_navigation_start(&mut self, time: u64) {
--- a/servo/components/script/dom/document.rs
+++ b/servo/components/script/dom/document.rs
@@ -92,17 +92,17 @@ use dom_struct::dom_struct;
 use encoding_rs::{Encoding, UTF_8};
 use euclid::Point2D;
 use html5ever::{LocalName, Namespace, QualName};
 use hyper::header::{Header, SetCookie};
 use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcSender};
 use js::jsapi::{JSContext, JSRuntime};
 use js::jsapi::JS_GetRuntime;
-use metrics::{InteractiveFlag, InteractiveMetrics, InteractiveWindow, ProfilerMetadataFactory};
+use metrics::{InteractiveFlag, InteractiveMetrics, InteractiveWindow, ProfilerMetadataFactory, ProgressiveWebMetric};
 use msg::constellation_msg::{BrowsingContextId, Key, KeyModifiers, KeyState, TopLevelBrowsingContextId};
 use net_traits::{FetchResponseMsg, IpcSend, ReferrerPolicy};
 use net_traits::CookieSource::NonHTTP;
 use net_traits::CoreResourceMsg::{GetCookiesForUrl, SetCookiesForUrl};
 use net_traits::pub_domains::is_pub_domain;
 use net_traits::request::RequestInit;
 use net_traits::response::HttpsState;
 use num_traits::ToPrimitive;
@@ -1906,16 +1906,20 @@ impl Document {
     pub fn get_dom_loading(&self) -> u64 {
         self.dom_loading.get()
     }
 
     pub fn get_dom_interactive(&self) -> u64 {
         self.dom_interactive.get()
     }
 
+    pub fn set_navigation_start(&self, navigation_start: u64) {
+        self.interactive_time.borrow_mut().set_navigation_start(navigation_start);
+    }
+
     pub fn get_interactive_metrics(&self) -> Ref<InteractiveMetrics> {
         self.interactive_time.borrow()
     }
 
     pub fn has_recorded_tti_metric(&self) -> bool {
         self.get_interactive_metrics().get_tti().is_some()
     }
 
@@ -1935,25 +1939,26 @@ impl Document {
         self.load_event_start.get()
     }
 
     pub fn get_load_event_end(&self) -> u64 {
         self.load_event_end.get()
     }
 
     pub fn start_tti(&self) {
-        self.tti_window.borrow_mut().start_window();
+        if self.get_interactive_metrics().needs_tti() {
+            self.tti_window.borrow_mut().start_window();
+        }
     }
 
     /// check tti for this document
     /// if it's been 10s since this doc encountered a task over 50ms, then we consider the
     /// main thread available and try to set tti
     pub fn record_tti_if_necessary(&self) {
         if self.has_recorded_tti_metric() { return; }
-
         if self.tti_window.borrow().needs_check() {
             self.get_interactive_metrics().maybe_set_tti(self,
                 InteractiveFlag::TimeToInteractive(self.tti_window.borrow().get_start()));
         }
     }
 
     // https://html.spec.whatwg.org/multipage/#fire-a-focus-event
     fn fire_focus_event(&self, focus_event_type: FocusEventType, node: &Node, related_target: Option<&EventTarget>) {
--- a/servo/components/script/script_thread.rs
+++ b/servo/components/script/script_thread.rs
@@ -1243,16 +1243,19 @@ impl ScriptThread {
             profile(profiler_cat, None, self.time_profiler_chan.clone(), f)
         } else {
             f()
         };
         let end = precise_time_ns();
         for (doc_id, doc) in self.documents.borrow().iter() {
            if let Some(pipeline_id) = pipeline_id {
                 if pipeline_id == doc_id && end - start > MAX_TASK_NS {
+                    if opts::get().print_pwm {
+                        println!("Task took longer than max allowed ({:?}) {:?}", category, end - start);
+                    }
                     doc.start_tti();
                 }
             }
             doc.record_tti_if_necessary();
         }
         value
     }
 
@@ -2224,16 +2227,17 @@ impl ScriptThread {
             .unwrap();
 
         // Notify devtools that a new script global exists.
         self.notify_devtools(document.Title(), final_url.clone(), (incomplete.pipeline_id, None));
 
         let parse_input = DOMString::new();
 
         document.set_https_state(metadata.https_state);
+        document.set_navigation_start(incomplete.navigation_start_precise);
 
         if is_html_document == IsHTMLDocument::NonHTMLDocument {
             ServoParser::parse_xml_document(&document, parse_input, final_url);
         } else {
             ServoParser::parse_html_document(&document, parse_input, final_url);
         }
 
         if incomplete.activity == DocumentActivity::FullyActive {