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

Bug 1727877 - Only allocate JS frame buffer when thread is being profiled - r=canaltinova 

The buffer was previously allocated as soon as a JSContext was present, meaning that some memory was used even when the profiler was not running, which is the case most of the time for most users.
This saves some memory, at the cost of having to lock the per-thread ThreadRegistration data when sampling a thread with a JSContext. This should have low contention risk, because it's mostly used when sampling on the thread, while the periodic sampler uses its own buffer so it doesn't need to lock the per-thread data.

Differential Revision: https://phabricator.services.mozilla.com/D123910
parent 9f1a306d

tools/profiler/core/ProfilerThreadRegistrationData.cpp

+23 −6
Original line number Diff line number Diff line
@@ -108,8 +108,16 @@ void ThreadRegistrationLockedRWFromAnyThread:: 
  MOZ_ASSERT(aProfiledThreadData);

  mProfiledThreadData = aProfiledThreadData;



  if (mJSContext) {

    // The thread is now being profiled, and we already have a JSContext,

    // allocate a JsFramesBuffer to allow profiler-unlocked on-thread sampling.

    MOZ_ASSERT(!mJsFrameBuffer);

    mJsFrameBuffer = new JsFrame[MAX_JS_FRAMES];

  }



  // Check invariants.

  MOZ_ASSERT(mIsBeingProfiled == !!mProfiledThreadData);

  MOZ_ASSERT((mJSContext && mIsBeingProfiled) == !!mJsFrameBuffer);

}



void ThreadRegistrationLockedRWFromAnyThread::
@@ -117,8 +125,14 @@ void ThreadRegistrationLockedRWFromAnyThread:: 
  mIsBeingProfiled = false;

  mProfiledThreadData = nullptr;



  if (mJsFrameBuffer) {

    delete[] mJsFrameBuffer;

    mJsFrameBuffer = nullptr;

  }



  // Check invariants.

  MOZ_ASSERT(mIsBeingProfiled == !!mProfiledThreadData);

  MOZ_ASSERT((mJSContext && mIsBeingProfiled) == !!mJsFrameBuffer);

}



void ThreadRegistrationLockedRWOnThread::SetJSContext(JSContext* aJSContext) {
@@ -126,17 +140,20 @@ void ThreadRegistrationLockedRWOnThread::SetJSContext(JSContext* aJSContext) { 


  mJSContext = aJSContext;



  // We now have a JSContext, allocate a JsFramesBuffer to allow unlocked

  // on-thread sampling.

  if (mProfiledThreadData) {

    MOZ_ASSERT(mIsBeingProfiled == !!mProfiledThreadData);

    // We now have a JSContext, and the thread is already being profiled,

    // allocate a JsFramesBuffer to allow profiler-unlocked on-thread sampling.

    MOZ_ASSERT(!mJsFrameBuffer);

    mJsFrameBuffer = new JsFrame[MAX_JS_FRAMES];

  }



  // We give the JS engine a non-owning reference to the ProfilingStack. It's

  // important that the JS engine doesn't touch this once the thread dies.

  js::SetContextProfilingStack(aJSContext, &ProfilingStackRef());



  // Check invariants.

  MOZ_ASSERT(!!mJSContext == !!mJsFrameBuffer);

  MOZ_ASSERT((mJSContext && mIsBeingProfiled) == !!mJsFrameBuffer);

}



void ThreadRegistrationLockedRWOnThread::ClearJSContext() {
@@ -148,7 +165,7 @@ void ThreadRegistrationLockedRWOnThread::ClearJSContext() { 
  }



  // Check invariants.

  MOZ_ASSERT(!!mJSContext == !!mJsFrameBuffer);

  MOZ_ASSERT((mJSContext && mIsBeingProfiled) == !!mJsFrameBuffer);

}



void ThreadRegistrationLockedRWOnThread::PollJSSampling() {

tools/profiler/core/platform.cpp

+41 −7
Original line number Diff line number Diff line
@@ -2339,9 +2339,25 @@ static void DoSyncSample( 
  TimeDuration delta = aNow - CorePS::ProcessStartTime();

  aBuffer.AddEntry(ProfileBufferEntry::Time(delta.ToMilliseconds()));



  if (!aThreadData.GetJSContext()) {

    // No JSContext, there is no JS frame buffer (and no need for it).

    DoSharedSample(/* aIsSynchronous = */ true, aFeatures, aThreadData,

                 aThreadData.GetJsFrameBuffer(), aRegs, samplePos,

                   /* aJsFrames = */ nullptr, aRegs, samplePos,

                   bufferRangeStart, aBuffer, aCaptureOptions);

  } else {

    // JSContext is present, we need to lock the thread data to access the JS

    // frame buffer.

    ThreadRegistration::WithOnThreadRef([&](ThreadRegistration::OnThreadRef

                                                aOnThreadRef) {

      aOnThreadRef.WithConstLockedRWOnThread(

          [&](const ThreadRegistration::LockedRWOnThread& aLockedThreadData) {

            DoSharedSample(/* aIsSynchronous = */ true, aFeatures, aThreadData,

                           aLockedThreadData.GetJsFrameBuffer(), aRegs,

                           samplePos, bufferRangeStart, aBuffer,

                           aCaptureOptions);

          });

    });

  }

}



// Writes the components of a periodic sample to ActivePS's ProfileBuffer.
@@ -5773,9 +5789,26 @@ void profiler_suspend_and_sample_thread(ProfilerThreadId aThreadId, 
                // TODO: Remove this lock when on-thread sampling doesn't

                // require it anymore.

                PSAutoLock lock;

                if (!aThreadData.GetJSContext()) {

                  // No JSContext, there is no JS frame buffer (and no need for

                  // it).

                  profiler_suspend_and_sample_thread(

                    lock, aThreadData, aThreadData.GetJsFrameBuffer(), true,

                    aFeatures, aCollector, aSampleNative);

                      lock, aThreadData, /* aJsFrames = */ nullptr,

                      /* aIsSynchronous = */ true, aFeatures, aCollector,

                      aSampleNative);

                } else {

                  // JSContext is present, we need to lock the thread data to

                  // access the JS frame buffer.

                  aOnThreadRef.WithConstLockedRWOnThread(

                      [&](const ThreadRegistration::LockedRWOnThread&

                              aLockedThreadData) {

                        profiler_suspend_and_sample_thread(

                            lock, aThreadData,

                            aLockedThreadData.GetJsFrameBuffer(),

                            /* aIsSynchronous = */ true, aFeatures, aCollector,

                            aSampleNative);

                      });

                }

              });

        });

  } else {
@@ -5788,7 +5821,8 @@ void profiler_suspend_and_sample_thread(ProfilerThreadId aThreadId, 
                      aThreadData) {

                JsFrameBuffer& jsFrames = CorePS::JsFrames(lock);

                profiler_suspend_and_sample_thread(lock, aThreadData, jsFrames,

                                                   false, aFeatures, aCollector,

                                                   /* aIsSynchronous = */ false,

                                                   aFeatures, aCollector,

                                                   aSampleNative);

              });

        });

tools/profiler/public/ProfilerThreadRegistrationData.h

+6 −5
Original line number Diff line number Diff line
@@ -110,8 +110,8 @@ class ThreadRegistrationData { 
  // Written from thread, read from thread and suspended thread.

  JSContext* mJSContext = nullptr;



  // If mJSContext is not null, this points at the start of a JsFrameBuffer to

  // be used for on-thread synchronous sampling.

  // If mJSContext is not null AND the thread is being profiled, this points at

  // the start of a JsFrameBuffer to be used for on-thread synchronous sampling.

  JsFrame* mJsFrameBuffer = nullptr;



  // The profiler needs to start and stop JS sampling of JS threads at various
@@ -362,9 +362,6 @@ class ThreadRegistrationUnlockedReaderAndAtomicRWOnThread 


  [[nodiscard]] JSContext* GetJSContext() const { return mJSContext; }



  // Not null when JSContext is not null. Points at the start of JsFrameBuffer.

  [[nodiscard]] JsFrame* GetJsFrameBuffer() const { return mJsFrameBuffer; }



 protected:

  ThreadRegistrationUnlockedReaderAndAtomicRWOnThread(const char* aName,

                                                      const void* aStackTop)
@@ -382,6 +379,10 @@ class ThreadRegistrationLockedRWFromAnyThread 
      ProfiledThreadData* aProfiledThreadData, const PSAutoLock&);

  void ClearIsBeingProfiledAndProfiledThreadData(const PSAutoLock&);



  // Not null when JSContext is not null AND this thread is being profiled.

  // Points at the start of JsFrameBuffer.

  [[nodiscard]] JsFrame* GetJsFrameBuffer() const { return mJsFrameBuffer; }



  [[nodiscard]] const nsCOMPtr<nsIEventTarget> GetEventTarget() const {

    return mThread;

  }

tools/profiler/tests/gtest/GeckoProfiler.cpp

+1 −1
Original line number Diff line number Diff line
@@ -301,7 +301,6 @@ static void TestConstUnlockedReaderAndAtomicRWOnThread( 
                                       aThreadId);



  EXPECT_EQ(aData.GetJSContext(), nullptr);

  EXPECT_EQ(aData.GetJsFrameBuffer(), nullptr);

};



static void TestUnlockedRWForLockedProfiler(
@@ -343,6 +342,7 @@ static void TestConstLockedRWFromAnyThread( 
                                             aAfterRegistration, aOnStackObject,

                                             aThreadId);



  EXPECT_EQ(aData.GetJsFrameBuffer(), nullptr);

  EXPECT_EQ(aData.GetEventTarget(), nullptr);

};