Auto merge of #6608 - nnethercote:tweak-reporter-registration, r=glennw

Register/unregister memory reporters in a better place.

By doing this on either side of the call to the relevant tasks' start()
method, we don't need to store the mem::ProfilerChan or the reporter
name in the task itself.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6608)
<!-- Reviewable:end -->

2 files changed, 23 insertions, 32 deletions

diff --git a/components/gfx/paint_task.rs b/components/gfx/paint_task.rs
index 308b270326c..4083fd05aa2 100644
--- a/components/gfx/paint_task.rs
+++ b/components/gfx/paint_task.rs
@@ -116,12 +116,6 @@ pub struct PaintTask<C> {
     /// A channel to the time profiler.
     time_profiler_chan: time::ProfilerChan,
 
-    /// A channel to the memory profiler.
-    mem_profiler_chan: mem::ProfilerChan,
-
-    /// The name used for the task's memory reporter.
-    pub reporter_name: String,
-
     /// The root stacking context sent to us by the layout thread.
     root_stacking_context: Option<Arc<StackingContext>>,
 
@@ -170,12 +164,6 @@ impl<C> PaintTask<C> where C: PaintListener + Send + 'static {
                                                               font_cache_task,
                                                               time_profiler_chan.clone());
 
-                // Register this thread as a memory reporter, via its own channel.
-                let reporter = box chan.clone();
-                let reporter_name = format!("paint-reporter-{}", id.0);
-                mem_profiler_chan.send(mem::ProfilerMsg::RegisterReporter(reporter_name.clone(),
-                                                                          reporter));
-
                 // FIXME: rust/#5967
                 let mut paint_task = PaintTask {
                     id: id,
@@ -184,8 +172,6 @@ impl<C> PaintTask<C> where C: PaintListener + Send + 'static {
                     compositor: compositor,
                     constellation_chan: constellation_chan,
                     time_profiler_chan: time_profiler_chan,
-                    mem_profiler_chan: mem_profiler_chan,
-                    reporter_name: reporter_name,
                     root_stacking_context: None,
                     paint_permission: false,
                     current_epoch: None,
@@ -193,8 +179,17 @@ impl<C> PaintTask<C> where C: PaintListener + Send + 'static {
                     canvas_map: HashMap::new()
                 };
 
+                // Register this thread as a memory reporter, via its own channel.
+                let reporter = box chan.clone();
+                let reporter_name = format!("paint-reporter-{}", id.0);
+                let msg = mem::ProfilerMsg::RegisterReporter(reporter_name.clone(), reporter);
+                mem_profiler_chan.send(msg);
+
                 paint_task.start();
 
+                let msg = mem::ProfilerMsg::UnregisterReporter(reporter_name);
+                mem_profiler_chan.send(msg);
+
                 // Tell all the worker threads to shut down.
                 for worker_thread in paint_task.worker_threads.iter_mut() {
                     worker_thread.exit()
@@ -270,9 +265,6 @@ impl<C> PaintTask<C> where C: PaintListener + Send + 'static {
                     // FIXME(njn): should eventually measure the paint task.
                 }
                 Msg::Exit(response_channel, _) => {
-                    let msg = mem::ProfilerMsg::UnregisterReporter(self.reporter_name.clone());
-                    self.mem_profiler_chan.send(msg);
-
                     // Ask the compositor to remove any layers it is holding for this paint task.
                     // FIXME(mrobinson): This can probably move back to the constellation now.
                     self.compositor.notify_paint_task_exiting(self.id);
diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs
index 860762aecca..36fbaa88383 100644
--- a/components/layout/layout_task.rs
+++ b/components/layout/layout_task.rs
@@ -185,9 +185,6 @@ pub struct LayoutTask {
     /// The channel on which messages can be sent to the memory profiler.
     pub mem_profiler_chan: mem::ProfilerChan,
 
-    /// The name used for the task's memory reporter.
-    pub reporter_name: String,
-
     /// The channel on which messages can be sent to the image cache.
     pub image_cache_task: ImageCacheTask,
 
@@ -224,17 +221,18 @@ impl LayoutTaskFactory for LayoutTask {
               image_cache_task: ImageCacheTask,
               font_cache_task: FontCacheTask,
               time_profiler_chan: time::ProfilerChan,
-              memory_profiler_chan: mem::ProfilerChan,
+              mem_profiler_chan: mem::ProfilerChan,
               shutdown_chan: Sender<()>) {
         let ConstellationChan(con_chan) = constellation_chan.clone();
         spawn_named_with_send_on_failure(format!("LayoutTask {:?}", id), task_state::LAYOUT, move || {
             { // Ensures layout task is destroyed before we send shutdown message
                 let sender = chan.sender();
+                let layout_chan = LayoutChan(sender);
                 let layout = LayoutTask::new(id,
                                              url,
                                              is_iframe,
                                              chan.receiver(),
-                                             LayoutChan(sender),
+                                             layout_chan.clone(),
                                              pipeline_port,
                                              constellation_chan,
                                              script_chan,
@@ -242,8 +240,18 @@ impl LayoutTaskFactory for LayoutTask {
                                              image_cache_task,
                                              font_cache_task,
                                              time_profiler_chan,
-                                             memory_profiler_chan);
+                                             mem_profiler_chan.clone());
+
+                // Register this thread as a memory reporter, via its own channel.
+                let reporter = box layout_chan.clone();
+                let reporter_name = format!("layout-reporter-{}", id.0);
+                let msg = mem::ProfilerMsg::RegisterReporter(reporter_name.clone(), reporter);
+                mem_profiler_chan.send(msg);
+
                 layout.start();
+
+                let msg = mem::ProfilerMsg::UnregisterReporter(reporter_name);
+                mem_profiler_chan.send(msg);
             }
             shutdown_chan.send(()).unwrap();
         }, ConstellationMsg::Failure(failure_msg), con_chan);
@@ -307,11 +315,6 @@ impl LayoutTask {
             None
         };
 
-        // Register this thread as a memory reporter, via its own channel.
-        let reporter = box chan.clone();
-        let reporter_name = format!("layout-reporter-{}", id.0);
-        mem_profiler_chan.send(mem::ProfilerMsg::RegisterReporter(reporter_name.clone(), reporter));
-
         // Create the channel on which new animations can be sent.
         let (new_animations_sender, new_animations_receiver) = channel();
         let (image_cache_sender, image_cache_receiver) = channel();
@@ -337,7 +340,6 @@ impl LayoutTask {
             paint_chan: paint_chan,
             time_profiler_chan: time_profiler_chan,
             mem_profiler_chan: mem_profiler_chan,
-            reporter_name: reporter_name,
             image_cache_task: image_cache_task.clone(),
             font_cache_task: font_cache_task,
             first_reflow: Cell::new(true),
@@ -673,9 +675,6 @@ impl LayoutTask {
             LayoutTask::return_rw_data(possibly_locked_rw_data, rw_data);
         }
 
-        let msg = mem::ProfilerMsg::UnregisterReporter(self.reporter_name.clone());
-        self.mem_profiler_chan.send(msg);
-
         self.paint_chan.send(PaintMsg::Exit(Some(response_chan), exit_type));
         response_port.recv().unwrap()
     }