Commit c83e6a05 authored by Gerald Squelart's avatar Gerald Squelart
Browse files

Bug 1715922 - Windows timer resolution is enhanced by default, except if... 

Bug 1715922 - Windows timer resolution is enhanced by default, except if notimerresolutionchange feature is set - r=canaltinova

This reverses bug 1703410: By default the profiler now changes the timer resolution (normally 64Hz) when the requested sampling interval is lower than 10ms, to allow fast-enough sampling for most uses.

But since this can influence other timers in Firefox, it makes debugging some types of issues more difficult. To help with this, there is now a "No Timer Resolution change" on Windows, which prevents the profiler from changing the timer resolution, at a risk of slowing down sampling in some processes.

Differential Revision: https://phabricator.services.mozilla.com/D117626
parent f6de2de4

devtools/client/performance-new/utils.js

+9 −0
Original line number Diff line number Diff line
@@ -408,6 +408,15 @@ const featureDescriptions = [ 
    value: "audiocallbacktracing",

    title: "Trace real-time audio callbacks.",

  },

  {

    name: "No Timer Resolution Change",

    value: "notimerresolutionchange",

    title:

      "Do not enhance the timer resolution for sampling intervals < 10ms, to " +

      "avoid affecting timer-sensitive code. Warning: Sampling interval may " +

      "increase in some processes.",

    disabledReason: "Windows only.",

  },

];



module.exports = {

mozglue/baseprofiler/core/platform-linux-android.cpp

+2 −1
Original line number Diff line number Diff line
@@ -412,7 +412,8 @@ static void* ThreadEntry(void* aArg) { 


SamplerThread::SamplerThread(PSLockRef aLock, uint32_t aActivityGeneration,

                             double aIntervalMilliseconds,

                             bool aStackWalkEnabled)

                             bool aStackWalkEnabled,

                             bool aNoTimerResolutionChange)

    : mSampler(aLock),

      mActivityGeneration(aActivityGeneration),

      mIntervalMicroseconds(

mozglue/baseprofiler/core/platform-macos.cpp

+2 −1
Original line number Diff line number Diff line
@@ -180,7 +180,8 @@ static void* ThreadEntry(void* aArg) { 


SamplerThread::SamplerThread(PSLockRef aLock, uint32_t aActivityGeneration,

                             double aIntervalMilliseconds,

                             bool aStackWalkEnabled)

                             bool aStackWalkEnabled,

                             bool aNoTimerResolutionChange)

    : mSampler(aLock),

      mActivityGeneration(aActivityGeneration),

      mIntervalMicroseconds(

mozglue/baseprofiler/core/platform-win32.cpp

+13 −26
Original line number Diff line number Diff line
@@ -212,34 +212,23 @@ static unsigned int __stdcall ThreadEntry(void* aArg) { 
  return 0;

}



static bool ShouldAdjustTimerResolution() {

  // Read PROFILER_ADJUST_TIMER_RESOLUTION env-var once per process, so Firefox

  // should be restarted if it is changed.

  // This is to ensure that each timeBeginPeriod is always paired with a

  // timeEndPeriod.

  static bool shouldAdjustTimerResolution =

      static_cast<bool>(getenv("PROFILER_ADJUST_TIMER_RESOLUTION"));

  return shouldAdjustTimerResolution;

}



SamplerThread::SamplerThread(PSLockRef aLock, uint32_t aActivityGeneration,

                             double aIntervalMilliseconds,

                             bool aStackWalkEnabled)

                             bool aStackWalkEnabled,

                             bool aNoTimerResolutionChange)

    : mSampler(aLock),

      mActivityGeneration(aActivityGeneration),

      mIntervalMicroseconds(

          std::max(1, int(floor(aIntervalMilliseconds * 1000 + 0.5)))) {

  if (ShouldAdjustTimerResolution()) {

    // By default we'll not adjust the timer resolution which tends to be

    // around 16ms. However, if the requested interval is sufficiently low

    // we'll try to adjust the resolution to match.

    // This is only set if explicitly requested with e.g.:

    // `PROFILER_ADJUST_TIMER_RESOLUTION=1`, because this affects all timers

    // in Firefox, and could therefore hide issues while profiling.

    if (mIntervalMicroseconds < 10 * 1000) {

          std::max(1, int(floor(aIntervalMilliseconds * 1000 + 0.5)))),

      mNoTimerResolutionChange(aNoTimerResolutionChange) {

  if ((!aNoTimerResolutionChange) && (mIntervalMicroseconds < 10 * 1000)) {

    // By default the timer resolution (which tends to be 1/64Hz, around 16ms)

    // is not changed. However, if the requested interval is sufficiently low,

    // the resolution will be adjusted to match. Note that this affects all

    // timers in Firefox, and could therefore hide issues while profiling. This

    // change may be prevented with the "notimerresolutionchange" feature.

    ::timeBeginPeriod(mIntervalMicroseconds / 1000);

  }

  }



  // Create a new thread. It is important to use _beginthreadex() instead of

  // the Win32 function CreateThread(), because the CreateThread() does not
@@ -285,7 +274,7 @@ void SamplerThread::SleepMicro(uint32_t aMicroseconds) { 
}



void SamplerThread::Stop(PSLockRef aLock) {

  if (ShouldAdjustTimerResolution()) {

  if ((!mNoTimerResolutionChange) && (mIntervalMicroseconds < 10 * 1000)) {

    // Disable any timer resolution changes we've made. Do it now while

    // gPSMutex is locked, i.e. before any other SamplerThread can be created

    // and call ::timeBeginPeriod().
@@ -294,10 +283,8 @@ void SamplerThread::Stop(PSLockRef aLock) { 
    // because the next time the main loop of Run() iterates it won't get past

    // the mActivityGeneration check, and so it won't make any more ::Sleep()

    // calls.

    if (mIntervalMicroseconds < 10 * 1000) {

    ::timeEndPeriod(mIntervalMicroseconds / 1000);

  }

  }



  mSampler.Disable(aLock);

}

mozglue/baseprofiler/core/platform.cpp

+20 −10
Original line number Diff line number Diff line
@@ -224,6 +224,9 @@ static uint32_t AvailableFeatures() { 
  ProfilerFeature::ClearStackWalk(features);

#endif

  ProfilerFeature::ClearJSTracer(features);

#if !defined(GP_OS_windows)

  ProfilerFeature::ClearNoTimerResolutionChange(features);

#endif



  return features;

}
@@ -569,8 +572,8 @@ ProfileChunkedBuffer& profiler_get_core_buffer() { 
class SamplerThread;



static SamplerThread* NewSamplerThread(PSLockRef aLock, uint32_t aGeneration,

                                       double aInterval,

                                       bool aStackWalkEnabled);

                                       double aInterval, bool aStackWalkEnabled,

                                       bool aNoTimerResolutionChange);



struct LiveProfiledThreadData {

  RegisteredThread* mRegisteredThread;
@@ -680,9 +683,10 @@ class ActivePS { 
        // The new sampler thread doesn't start sampling immediately because the

        // main loop within Run() is blocked until this function's caller

        // unlocks gPSMutex.

        mSamplerThread(

            NewSamplerThread(aLock, mGeneration, aInterval,

                             ProfilerFeature::HasStackWalk(aFeatures))),

        mSamplerThread(NewSamplerThread(

            aLock, mGeneration, aInterval,

            ProfilerFeature::HasStackWalk(aFeatures),

            ProfilerFeature::HasNoTimerResolutionChange(aFeatures))),

        mIsPaused(false),

        mIsSamplingPaused(false)

#if defined(GP_OS_linux) || defined(GP_OS_freebsd)
@@ -2213,7 +2217,8 @@ class SamplerThread { 
 public:

  // Creates a sampler thread, but doesn't start it.

  SamplerThread(PSLockRef aLock, uint32_t aActivityGeneration,

                double aIntervalMilliseconds, bool aStackWalkEnabled);

                double aIntervalMilliseconds, bool aStackWalkEnabled,

                bool aNoTimerResolutionChange);

  ~SamplerThread();



  // This runs on (is!) the sampler thread.
@@ -2244,6 +2249,10 @@ class SamplerThread { 
  pthread_t mThread;

#endif



#if defined(GP_OS_windows)

  bool mNoTimerResolutionChange = true;

#endif



  SamplerThread(const SamplerThread&) = delete;

  void operator=(const SamplerThread&) = delete;

};
@@ -2252,9 +2261,10 @@ class SamplerThread { 
// ActivePS's constructor, but SamplerThread is defined after ActivePS. It

// could probably be removed by moving some code around.

static SamplerThread* NewSamplerThread(PSLockRef aLock, uint32_t aGeneration,

                                       double aInterval,

                                       bool aStackWalkEnabled) {

  return new SamplerThread(aLock, aGeneration, aInterval, aStackWalkEnabled);

                                       double aInterval, bool aStackWalkEnabled,

                                       bool aNoTimerResolutionChange) {

  return new SamplerThread(aLock, aGeneration, aInterval, aStackWalkEnabled,

                           aNoTimerResolutionChange);

}



// This function is the sampler thread.  This implementation is used for all