rts: Fix gc timing
authorDouglas Wilson <douglas.wilson@gmail.com>
Wed, 15 Nov 2017 16:40:54 +0000 (11:40 -0500)
committerBen Gamari <ben@smart-cactus.org>
Wed, 15 Nov 2017 19:18:29 +0000 (14:18 -0500)
We were accumulating the gc times of the previous gc.
`stats.gc.{cpu,elappsed}_ns` were being accumulated into
`stats.gc_{cpu,elapsed}_ns` before they were set.

There is also a change in that heap profiling will no longer cause gc
events to
be emitted.

Reviewers: bgamari, erikd, simonmar

Reviewed By: bgamari

Subscribers: rwbarton, thomie

GHC Trac Issues: #14257, #14445

Differential Revision: https://phabricator.haskell.org/D4184

rts/Stats.c

index 6a5f801..8f7865b 100644 (file)
@@ -310,6 +310,26 @@ stat_endGC (Capability *cap, gc_thread *gct,
     stats.gc.par_max_copied_bytes = par_max_copied * sizeof(W_);
     stats.gc.par_balanced_copied_bytes = par_balanced_copied * sizeof(W_);
 
+    bool stats_enabled =
+        RtsFlags.GcFlags.giveStats != NO_GC_STATS ||
+        rtsConfig.gcDoneHook != NULL;
+
+    if (stats_enabled
+      || RtsFlags.ProfFlags.doHeapProfile) // heap profiling needs GC_tot_time
+    {
+        // We only update the times when stats are explicitly enabled since
+        // getProcessTimes (e.g. requiring a system call) can be expensive on
+        // some platforms.
+        Time current_cpu, current_elapsed;
+        getProcessTimes(&current_cpu, &current_elapsed);
+        stats.cpu_ns = current_cpu - start_init_cpu;
+        stats.elapsed_ns = current_elapsed - start_init_elapsed;
+
+        stats.gc.sync_elapsed_ns =
+            gct->gc_start_elapsed - gct->gc_sync_start_elapsed;
+        stats.gc.elapsed_ns = current_elapsed - gct->gc_start_elapsed;
+        stats.gc.cpu_ns = current_cpu - gct->gc_start_cpu;
+    }
     // -------------------------------------------------
     // Update the cumulative stats
 
@@ -354,23 +374,8 @@ stat_endGC (Capability *cap, gc_thread *gct,
     // -------------------------------------------------
     // Do the more expensive bits only when stats are enabled.
 
-    if (RtsFlags.GcFlags.giveStats != NO_GC_STATS ||
-        rtsConfig.gcDoneHook != NULL ||
-        RtsFlags.ProfFlags.doHeapProfile) // heap profiling needs GC_tot_time
+    if (stats_enabled)
     {
-        // We only update the times when stats are explicitly enabled since
-        // getProcessTimes (e.g. requiring a system call) can be expensive on
-        // some platforms.
-        Time current_cpu, current_elapsed;
-        getProcessTimes(&current_cpu, &current_elapsed);
-        stats.cpu_ns = current_cpu - start_init_cpu;
-        stats.elapsed_ns = current_elapsed - start_init_elapsed;
-
-        stats.gc.sync_elapsed_ns =
-          gct->gc_start_elapsed - gct->gc_sync_start_elapsed;
-        stats.gc.elapsed_ns = current_elapsed - gct->gc_start_elapsed;
-        stats.gc.cpu_ns = current_cpu - gct->gc_start_cpu;
-
         // -------------------------------------------------
         // Emit events to the event log