Title: [202021] trunk/Source/_javascript_Core
Revision
202021
Author
sbar...@apple.com
Date
2016-06-13 19:29:26 -0700 (Mon, 13 Jun 2016)

Log Message

The sampling profiler should further protect itself against certain forms of sampling bias that arise due to the sampling interval being in sync with some other system process
https://bugs.webkit.org/show_bug.cgi?id=158678

Reviewed by Benjamin Poulain.

I first became aware of this problem when I read this paper:
http://plv.colorado.edu/papers/mytkowicz-pldi10.pdf

To provide background for this change, I'll quote a paragraph
from section 6.2:
"One statically sound method for collecting random samples is to collect a
sample at every t + r milliseconds, where t is the desired sampling interval
and r is a random number between −t and t. One might think that sampling every
t seconds is enough (i.e., drop the r component) but it is not: specifically,
if a profiler samples every t seconds, the sampling rate would be synchronized
with any program or system activity that occurs at regular time intervals [17].
For example, if the thread scheduler switches between threads every 10ms and our
sampling interval was also 10ms, then we may always take samples immediately after
a thread switch. Because performance is often different immediately after a thread
switch than at other points (e.g., due to cache and TLB warm-up effects) we would
get biased data. The random component, r, guards against such situations."

* runtime/SamplingProfiler.cpp:
(JSC::SamplingProfiler::timerLoop):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (202020 => 202021)


--- trunk/Source/_javascript_Core/ChangeLog	2016-06-14 02:27:08 UTC (rev 202020)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-06-14 02:29:26 UTC (rev 202021)
@@ -1,3 +1,30 @@
+2016-06-13  Saam Barati  <sbar...@apple.com>
+
+        The sampling profiler should further protect itself against certain forms of sampling bias that arise due to the sampling interval being in sync with some other system process
+        https://bugs.webkit.org/show_bug.cgi?id=158678
+
+        Reviewed by Benjamin Poulain.
+
+        I first became aware of this problem when I read this paper:
+        http://plv.colorado.edu/papers/mytkowicz-pldi10.pdf
+
+        To provide background for this change, I'll quote a paragraph
+        from section 6.2:
+        "One statically sound method for collecting random samples is to collect a
+        sample at every t + r milliseconds, where t is the desired sampling interval
+        and r is a random number between −t and t. One might think that sampling every
+        t seconds is enough (i.e., drop the r component) but it is not: specifically,
+        if a profiler samples every t seconds, the sampling rate would be synchronized
+        with any program or system activity that occurs at regular time intervals [17].
+        For example, if the thread scheduler switches between threads every 10ms and our
+        sampling interval was also 10ms, then we may always take samples immediately after
+        a thread switch. Because performance is often different immediately after a thread
+        switch than at other points (e.g., due to cache and TLB warm-up effects) we would
+        get biased data. The random component, r, guards against such situations."
+
+        * runtime/SamplingProfiler.cpp:
+        (JSC::SamplingProfiler::timerLoop):
+
 2016-06-13  Oliver Hunt  <oli...@apple.com>
 
         DFG Validation fails when performing a concatenation with only a single entry

Modified: trunk/Source/_javascript_Core/runtime/SamplingProfiler.cpp (202020 => 202021)


--- trunk/Source/_javascript_Core/runtime/SamplingProfiler.cpp	2016-06-14 02:27:08 UTC (rev 202020)
+++ trunk/Source/_javascript_Core/runtime/SamplingProfiler.cpp	2016-06-14 02:29:26 UTC (rev 202021)
@@ -48,6 +48,7 @@
 #include "VM.h"
 #include "VMEntryScope.h"
 #include <wtf/HashSet.h>
+#include <wtf/RandomNumber.h>
 #include <wtf/RefPtr.h>
 
 namespace JSC {
@@ -233,7 +234,13 @@
             m_lastTime = m_stopwatch->elapsedTime();
         }
 
-        std::this_thread::sleep_for(m_timingInterval - std::min(m_timingInterval, stackTraceProcessingTime));
+        // Read section 6.2 of this paper for more elaboration of why we add a random
+        // fluctuation here. The main idea is to prevent our timer from being in sync
+        // with some system process such as a scheduled context switch.
+        // http://plv.colorado.edu/papers/mytkowicz-pldi10.pdf
+        double randomSignedNumber = (randomNumber() * 2.0) - 1.0; // A random number between [-1, 1).
+        std::chrono::microseconds randomFluctuation = std::chrono::microseconds(static_cast<uint64_t>(randomSignedNumber * static_cast<double>(m_timingInterval.count()) * 0.20l));
+        std::this_thread::sleep_for(m_timingInterval - std::min(m_timingInterval, stackTraceProcessingTime) + randomFluctuation);
     }
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to