Auto merge of #23521 - oneturkmen:profile-remove-opts-get, r=jdm

Profile: removed opts::get()

<!-- Please describe your changes on the following line: -->
Removed opts::get() from `profile` component. **Note** that `profile_traits` component is the only component that uses single `opts::get().signpost` for IpcBytesReceiver and IpcReceiver structs, i.e. for recv() type method.

---
<!-- 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 *partially* #22854 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because these are cleanup 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/23521)
<!-- Reviewable:end -->

components/profile/heartbeats.rs

@@ -5,7 +5,6 @@
 use self::synchronized_heartbeat::{heartbeat_window_callback, lock_and_work};
 use heartbeats_simple::HeartbeatPow as Heartbeat;
 use profile_traits::time::ProfilerCategory;
-use servo_config::opts;
 use std::collections::HashMap;
 use std::env::var_os;
 use std::error::Error;
@@ -13,11 +12,15 @@ use std::fs::File;
 use std::path::Path;
 
 /// Initialize heartbeats
-pub fn init() {
+pub fn init(profile_heartbeats: bool) {
     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);
+            maybe_create_heartbeat(
+                &mut hbs,
+                ProfilerCategory::ApplicationHeartbeat,
+                profile_heartbeats,
+            );
             *hbs_opt = Some(Box::into_raw(hbs))
         }
     });
@@ -40,13 +43,13 @@ pub fn cleanup() {
 }
 
 /// Check if a heartbeat exists for the given category
-pub fn is_heartbeat_enabled(category: &ProfilerCategory) -> bool {
+pub fn is_heartbeat_enabled(category: &ProfilerCategory, profile_heartbeats: bool) -> bool {
     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)
+    is_enabled || is_create_heartbeat(category, profile_heartbeats)
 }
 
 /// Issue a heartbeat (if one exists) for the given category
@@ -56,12 +59,13 @@ pub fn maybe_heartbeat(
     end_time: u64,
     start_energy: u64,
     end_energy: u64,
+    profile_heartbeats: bool,
 ) {
     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());
+                    maybe_create_heartbeat(&mut (*hbs_ptr), category.clone(), profile_heartbeats);
                 }
                 if let Some(h) = (*hbs_ptr).get_mut(category) {
                     (*h).heartbeat(0, 1, start_time, end_time, start_energy, end_energy);
@@ -73,9 +77,8 @@ pub fn maybe_heartbeat(
 
 // TODO(cimes): Android doesn't really do environment variables. Need a better way to configure dynamically.
 
-fn is_create_heartbeat(category: &ProfilerCategory) -> bool {
+fn is_create_heartbeat(category: &ProfilerCategory, profile_heartbeats: bool) -> bool {
-    opts::get().profile_heartbeats ||
+    profile_heartbeats || var_os(format!("SERVO_HEARTBEAT_ENABLE_{:?}", category)).is_some()
-        var_os(format!("SERVO_HEARTBEAT_ENABLE_{:?}", category)).is_some()
 }
 
 fn open_heartbeat_log<P: AsRef<Path>>(name: P) -> Option<File> {
@@ -113,8 +116,9 @@ fn get_heartbeat_window_size(category: &ProfilerCategory) -> usize {
 fn maybe_create_heartbeat(
     hbs: &mut HashMap<ProfilerCategory, Heartbeat>,
     category: ProfilerCategory,
+    profile_heartbeats: bool,
 ) {
-    if is_create_heartbeat(&category) {
+    if is_create_heartbeat(&category, profile_heartbeats) {
         // get optional log file
         let logfile: Option<File> = get_heartbeat_log(&category);
         // window size

components/profile/time.rs

@@ -172,10 +172,15 @@ pub struct Profiler {
     pub last_msg: Option<ProfilerMsg>,
     trace: Option<TraceDump>,
     blocked_layout_queries: HashMap<String, u32>,
+    profile_heartbeats: bool,
 }
 
 impl Profiler {
-    pub fn create(output: &Option<OutputOptions>, file_path: Option<String>) -> ProfilerChan {
+    pub fn create(
+        output: &Option<OutputOptions>,
+        file_path: Option<String>,
+        profile_heartbeats: bool,
+    ) -> ProfilerChan {
         let (chan, port) = ipc::channel().unwrap();
         match *output {
             Some(ref option) => {
@@ -185,7 +190,8 @@ impl Profiler {
                     .name("Time profiler".to_owned())
                     .spawn(move || {
                         let trace = file_path.as_ref().and_then(|p| TraceDump::new(p).ok());
-                        let mut profiler = Profiler::new(port, trace, Some(outputoption));
+                        let mut profiler =
+                            Profiler::new(port, trace, Some(outputoption), profile_heartbeats);
                         profiler.start();
                     })
                     .expect("Thread spawning failed");
@@ -217,7 +223,7 @@ impl Profiler {
                         .name("Time profiler".to_owned())
                         .spawn(move || {
                             let trace = file_path.as_ref().and_then(|p| TraceDump::new(p).ok());
-                            let mut profiler = Profiler::new(port, trace, None);
+                            let mut profiler = Profiler::new(port, trace, None, profile_heartbeats);
                             profiler.start();
                         })
                         .expect("Thread spawning failed");
@@ -240,12 +246,16 @@ impl Profiler {
             },
         }
 
-        heartbeats::init();
+        heartbeats::init(profile_heartbeats);
         let profiler_chan = ProfilerChan(chan);
 
         // only spawn the application-level profiler thread if its heartbeat is enabled
-        let run_ap_thread =
+        let run_ap_thread = move || {
-            || heartbeats::is_heartbeat_enabled(&ProfilerCategory::ApplicationHeartbeat);
+            heartbeats::is_heartbeat_enabled(
+                &ProfilerCategory::ApplicationHeartbeat,
+                profile_heartbeats,
+            )
+        };
         if run_ap_thread() {
             let profiler_chan = profiler_chan.clone();
             // min of 1 heartbeat/sec, max of 20 should provide accurate enough power/energy readings
@@ -299,6 +309,7 @@ impl Profiler {
         port: IpcReceiver<ProfilerMsg>,
         trace: Option<TraceDump>,
         output: Option<OutputOptions>,
+        profile_heartbeats: bool,
     ) -> Profiler {
         Profiler {
             port: port,
@@ -307,6 +318,7 @@ impl Profiler {
             last_msg: None,
             trace: trace,
             blocked_layout_queries: HashMap::new(),
+            profile_heartbeats,
         }
     }
 
@@ -325,7 +337,7 @@ impl Profiler {
     fn handle_msg(&mut self, msg: ProfilerMsg) -> bool {
         match msg.clone() {
             ProfilerMsg::Time(k, t, e) => {
-                heartbeats::maybe_heartbeat(&k.0, t.0, t.1, e.0, e.1);
+                heartbeats::maybe_heartbeat(&k.0, t.0, t.1, e.0, e.1, self.profile_heartbeats);
                 if let Some(ref mut trace) = self.trace {
                     trace.write_one(&k, t, e);
                 }

components/servo/lib.rs

@@ -224,6 +224,7 @@ where
         let time_profiler_chan = profile_time::Profiler::create(
             &opts.time_profiling,
             opts.time_profiler_trace_path.clone(),
+            opts.profile_heartbeats,
         );
         let mem_profiler_chan = profile_mem::Profiler::create(opts.mem_profiler_period);
         let debugger_chan = opts.debugger_port.map(|port| debugger::start_server(port));

tests/unit/profile/time.rs

@@ -12,7 +12,7 @@ use std::time::Duration;
 
 #[test]
 fn time_profiler_smoke_test() {
-    let chan = time::Profiler::create(&None, None);
+    let chan = time::Profiler::create(&None, None, false);
     assert!(true, "Can create the profiler thread");
 
     let (ipcchan, _ipcport) = ipc::channel().unwrap();
@@ -45,7 +45,7 @@ fn time_profiler_stats_test() {
 
 #[test]
 fn channel_profiler_test() {
-    let chan = time::Profiler::create(&Some(OutputOptions::Stdout(5.0)), None);
+    let chan = time::Profiler::create(&Some(OutputOptions::Stdout(5.0)), None, false);
     let (profiled_sender, profiled_receiver) = ProfiledIpc::channel(chan.clone()).unwrap();
     thread::spawn(move || {
         thread::sleep(Duration::from_secs(2));
@@ -70,7 +70,7 @@ fn channel_profiler_test() {
 
 #[test]
 fn bytes_channel_profiler_test() {
-    let chan = time::Profiler::create(&Some(OutputOptions::Stdout(5.0)), None);
+    let chan = time::Profiler::create(&Some(OutputOptions::Stdout(5.0)), None, false);
     let (profiled_sender, profiled_receiver) = ProfiledIpc::bytes_channel(chan.clone()).unwrap();
     thread::spawn(move || {
         thread::sleep(Duration::from_secs(2));