Bug 1325299 - If cyclesDelta > totalCyclesDelta, reset data without comitting instead of failing assertions. r=Yoric
☠☠ backed out by 41d8ef56d03b ☠ ☠
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Thu, 12 Jan 2017 22:22:18 +0900
changeset 377583 cac5baad14a1afc0534c083191cad8bfd5c6429e
parent 377582 933d06e4b5673ecf5fdc15a00f1c49e0d4acda4a
child 377584 10e2e2b01b66cc98c97f5616f3ff7cef98bf1bb1
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs1325299
milestone53.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
Bug 1325299 - If cyclesDelta > totalCyclesDelta, reset data without comitting instead of failing assertions. r=Yoric - `totalCyclesDelta` is incremented whenever there is CPU usage in the topmost compartment *and* the execution of the topmost compartment stops on the same core as it started; - each individual `cyclesDelta` is incremented whenever there is CPU usage in a compartment *and* the execution of the compartment stops on the same core as it started; - however, with previous versions of Windows, the function to identify a core was not available, so the check was #ifdef-ed away. It is therefore entirely possible that, at some point during the execution of a mochitest, the thread is rescheduled to another core in a way such that at least one compartment executes entirely on a core but the topmost compartment starts and stops on a different core. Given that we're running on VMs that presumably run on timeshared servers, reschedulings are bound to be frequent, so it's hardly surprising that this always happens during the execution of mochitests. The simplest would probably be to throw away results if `cyclesDelta > totalCyclesDelta` for any of `cyclesDelta`. We should check if this happens and, if so, reset stuff without actually committing data. MozReview-Commit-ID: 3w2D1gtW4AQ
toolkit/components/perfmonitoring/nsPerformanceStats.cpp
--- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp
+++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp
@@ -1214,18 +1214,17 @@ nsPerformanceStatsService::CommitGroup(u
     return;
   }
 
   // When we add a group as changed, we immediately set its
   // `recentTicks` from 0 to 1.  If we have `ticksDelta == 0` at
   // this stage, we have already called `resetRecentData` but we
   // haven't removed it from the list.
   MOZ_ASSERT(ticksDelta != 0);
-  MOZ_ASSERT(cyclesDelta <= totalCyclesDelta);
-  if (cyclesDelta == 0 || totalCyclesDelta == 0) {
+  if (cyclesDelta > totalCyclesDelta || cyclesDelta == 0 || totalCyclesDelta == 0) {
     // Nothing useful, don't commit.
     return;
   }
 
   double proportion = (double)cyclesDelta / (double)totalCyclesDelta;
   MOZ_ASSERT(proportion <= 1);
 
   const uint64_t userTimeDelta = proportion * totalUserTimeDelta;