servo: Merge #15458 - Update heartbeats-simple dependencies for bug fixes (from connorimes:hbs-0.4); r=emilio
authorConnor Imes <connor.k.imes@gmail.com>
Wed, 22 Feb 2017 09:05:37 -0800
changeset 373343 2e2093189f5549660f99d09e0d5519865a2ede4f
parent 373342 1dcde4d265a4a7fdd9ad86bd216fd057670c74cb
child 373344 9f5fd56ce6ce81afc6d3b6a453aca05118fc2886
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone54.0a1
servo: Merge #15458 - Update heartbeats-simple dependencies for bug fixes (from connorimes:hbs-0.4); r=emilio <!-- Please describe your changes on the following line: --> Updates heartbeats-simple dependencies for some bug fixes in native code (primarily for Windows). Now we create heartbeats as needed so we don't have to maintain a hardcoded list which keeps getting out of sync. --- <!-- 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 #15471 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because it updates a dependency and performs some code maintenance. <!-- 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: 2bfea912dccd5e76e062143f3d48d2a66118fc59
servo/components/profile/heartbeats.rs
servo/tests/heartbeats/characterize.py
--- a/servo/components/profile/heartbeats.rs
+++ b/servo/components/profile/heartbeats.rs
@@ -1,116 +1,81 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-
 use heartbeats_simple::HeartbeatPow as Heartbeat;
-use heartbeats_simple::HeartbeatPowContext as HeartbeatContext;
 use profile_traits::time::ProfilerCategory;
+use self::synchronized_heartbeat::{heartbeat_window_callback, lock_and_work};
 use servo_config::opts;
 use std::collections::HashMap;
 use std::env::var_os;
 use std::error::Error;
 use std::fs::File;
-use std::mem;
 use std::path::Path;
 
-
-static mut HBS: Option<*mut HashMap<ProfilerCategory, Heartbeat>> = None;
-
 /// Initialize heartbeats
 pub fn init() {
-    let mut hbs: HashMap<ProfilerCategory, Heartbeat> = HashMap::new();
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::Compositing);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutPerform);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutStyleRecalc);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutTextShaping);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutRestyleDamagePropagation);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutNonIncrementalReset);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutSelectorMatch);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutTreeBuilder);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutDamagePropagate);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutGeneratedContent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutDisplayListSorting);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutFloatPlacementSpeculation);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutMain);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutStoreOverflow);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutParallelWarmup);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::LayoutDispListBuild);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::NetHTTPRequestResponse);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::PaintingPerTile);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::PaintingPrepBuff);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::Painting);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ImageDecoding);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ImageSaving);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptAttachLayout);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptConstellationMsg);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptDevtoolsMsg);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptDocumentEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptDomEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptEvaluate);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptFileRead);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptImageCacheMsg);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptInputEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptNetworkEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptParseHTML);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptPlannedNavigation);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptResize);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptSetScrollState);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptSetViewport);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptTimerEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptStylesheetLoad);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptUpdateReplacedElement);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptWebSocketEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptWorkerEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptServiceWorkerEvent);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ScriptParseXML);
-    maybe_create_heartbeat(&mut hbs, ProfilerCategory::ApplicationHeartbeat);
-    unsafe {
-        HBS = Some(mem::transmute(Box::new(hbs)));
-    }
+    lock_and_work(|hbs_opt|
+        if hbs_opt.is_none() {
+            let mut hbs: Box<HashMap<ProfilerCategory, Heartbeat>> = Box::new(HashMap::new());
+            maybe_create_heartbeat(&mut hbs, ProfilerCategory::ApplicationHeartbeat);
+            *hbs_opt = Some(Box::into_raw(hbs))
+        }
+    );
 }
 
 /// Log regmaining buffer data and cleanup heartbeats
 pub fn cleanup() {
-    unsafe {
-        if let Some(hbs) = HBS {
-            let mut h: Box<HashMap<ProfilerCategory, Heartbeat>> = mem::transmute(hbs);
-            for (_, mut v) in h.iter_mut() {
-                // log any remaining heartbeat records before dropping
-                log_heartbeat_records(v);
+    let hbs_opt_box: Option<Box<HashMap<ProfilerCategory, Heartbeat>>> = lock_and_work(|hbs_opt|
+        hbs_opt.take().map(|hbs_ptr|
+            unsafe {
+                Box::from_raw(hbs_ptr)
             }
-            h.clear();
+        )
+    );
+    if let Some(mut hbs) = hbs_opt_box {
+        for (_, mut v) in hbs.iter_mut() {
+            // log any remaining heartbeat records before dropping
+            log_heartbeat_records(v);
         }
-        HBS = None;
+        hbs.clear();
     }
 }
 
 /// Check if a heartbeat exists for the given category
 pub fn is_heartbeat_enabled(category: &ProfilerCategory) -> bool {
-    unsafe {
-        HBS.map_or(false, |m| (*m).contains_key(category))
-    }
+    let is_enabled = lock_and_work(|hbs_opt|
+        hbs_opt.map_or(false, |hbs_ptr|
+            unsafe {
+                (*hbs_ptr).contains_key(category)
+            }
+        )
+    );
+    is_enabled || is_create_heartbeat(category)
 }
 
 /// Issue a heartbeat (if one exists) for the given category
 pub fn maybe_heartbeat(category: &ProfilerCategory,
                        start_time: u64,
                        end_time: u64,
                        start_energy: u64,
                        end_energy: u64) {
-    unsafe {
-        if let Some(map) = HBS {
-            if let Some(mut h) = (*map).get_mut(category) {
-                (*h).heartbeat(0, 1, start_time, end_time, start_energy, end_energy);
+    lock_and_work(|hbs_opt|
+        if let Some(hbs_ptr) = *hbs_opt {
+            unsafe {
+                if !(*hbs_ptr).contains_key(category) {
+                    maybe_create_heartbeat(&mut (*hbs_ptr), category.clone());
+                }
+                if let Some(mut h) = (*hbs_ptr).get_mut(category) {
+                    (*h).heartbeat(0, 1, start_time, end_time, start_energy, end_energy);
+                }
             }
         }
-    }
+    );
 }
 
 // TODO(cimes): Android doesn't really do environment variables. Need a better way to configure dynamically.
 
 fn is_create_heartbeat(category: &ProfilerCategory) -> bool {
     opts::get().profile_heartbeats || var_os(format!("SERVO_HEARTBEAT_ENABLE_{:?}", category)).is_some()
 }
 
@@ -167,21 +132,46 @@ fn maybe_create_heartbeat(hbs: &mut Hash
 /// Log heartbeat records up to the buffer index
 fn log_heartbeat_records(hb: &mut Heartbeat) {
     match hb.log_to_buffer_index() {
         Ok(_) => (),
         Err(e) => warn!("Failed to write heartbeat log: {}", Error::description(&e)),
     }
 }
 
-/// Callback function used to log the window buffer.
-/// When this is called from native C, the heartbeat is safely locked
-extern fn heartbeat_window_callback(hb: *const HeartbeatContext) {
-    unsafe {
-        if let Some(map) = HBS {
-            for (_, v) in (*map).iter_mut() {
-                if &v.hb as *const HeartbeatContext == hb {
-                    log_heartbeat_records(v);
+mod synchronized_heartbeat {
+    use heartbeats_simple::HeartbeatPow as Heartbeat;
+    use heartbeats_simple::HeartbeatPowContext as HeartbeatContext;
+    use profile_traits::time::ProfilerCategory;
+    use std::collections::HashMap;
+    use std::sync::atomic::{ATOMIC_BOOL_INIT, AtomicBool, Ordering};
+    use super::log_heartbeat_records;
+
+    static mut HBS: Option<*mut HashMap<ProfilerCategory, Heartbeat>> = None;
+
+    // unfortunately can't encompass the actual hashmap in a Mutex (Heartbeat isn't Send/Sync), so we'll use a spinlock
+    static HBS_SPINLOCK: AtomicBool = ATOMIC_BOOL_INIT;
+
+    pub fn lock_and_work<F, R>(work: F) -> R
+        where F: FnOnce(&mut Option<*mut HashMap<ProfilerCategory, Heartbeat>>) -> R {
+        while HBS_SPINLOCK.compare_and_swap(false, true, Ordering::SeqCst) {}
+        let result = unsafe {
+            work(&mut HBS)
+        };
+        HBS_SPINLOCK.store(false, Ordering::SeqCst);
+        result
+    }
+
+    /// Callback function used to log the window buffer.
+    /// When this is called from native C, the heartbeat is safely locked internally and the global lock is held.
+    /// If calling from this file, you must already hold the global lock!
+    pub extern fn heartbeat_window_callback(hb: *const HeartbeatContext) {
+        unsafe {
+            if let Some(hbs_ptr) = HBS {
+                for (_, v) in (*hbs_ptr).iter_mut() {
+                    if &v.hb as *const HeartbeatContext == hb {
+                        log_heartbeat_records(v);
+                    }
                 }
             }
         }
     }
 }
--- a/servo/tests/heartbeats/characterize.py
+++ b/servo/tests/heartbeats/characterize.py
@@ -60,16 +60,19 @@ HEARTBEAT_PROFILER_CATEGORIES = [
     ("ScriptSetViewport", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ScriptTimerEvent", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ScriptStylesheetLoad", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ScriptUpdateReplacedElement", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ScriptWebSocketEvent", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ScriptWorkerEvent", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ScriptServiceWorkerEvent", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ScriptParseXML", HEARTBEAT_DEFAULT_WINDOW_SIZE),
+    ("ScriptEnterFullscreen", HEARTBEAT_DEFAULT_WINDOW_SIZE),
+    ("ScriptExitFullscreen", HEARTBEAT_DEFAULT_WINDOW_SIZE),
+    ("ScriptWebVREvent", HEARTBEAT_DEFAULT_WINDOW_SIZE),
     ("ApplicationHeartbeat", 100),
 ]
 ENERGY_READER_BIN = "energymon-file-provider"
 ENERGY_READER_TEMP_OUTPUT = "energymon.txt"
 SUMMARY_OUTPUT = "summary.txt"
 
 
 def get_command(build_target, layout_thread_count, renderer, page, profile):