Commit 5d1937d5 authored by Gerald Squelart's avatar Gerald Squelart
Browse files

Bug 1721569 - Handle different sizes of Profiler{Process,Thread}Id - r=florian 

Since ProfilerProcessId and ProfilerThreadId (and their NumberTypes) will potentially grow to 64 bits on some platforms (in a later patch), all code that uses them must be able to handle bigger types.

Differential Revision: https://phabricator.services.mozilla.com/D121049
parent c469a8f0

mozglue/baseprofiler/core/ProfileBuffer.cpp

+1 −1
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ ProfileBufferBlockIndex ProfileBuffer::AddThreadIdEntry( 
    ProfileChunkedBuffer& aProfileChunkedBuffer,

    BaseProfilerThreadId aThreadId) {

  return AddEntry(aProfileChunkedBuffer,

                  ProfileBufferEntry::ThreadId(aThreadId.ToNumber()));

                  ProfileBufferEntry::ThreadId(aThreadId));

}



uint64_t ProfileBuffer::AddThreadIdEntry(BaseProfilerThreadId aThreadId) {

mozglue/baseprofiler/core/ProfileBufferEntry.cpp

+17 −4
Original line number Diff line number Diff line
@@ -63,6 +63,14 @@ ProfileBufferEntry::ProfileBufferEntry(Kind aKind, uint64_t aUint64) 
  memcpy(mStorage, &aUint64, sizeof(aUint64));

}



ProfileBufferEntry::ProfileBufferEntry(Kind aKind,

                                       BaseProfilerThreadId aThreadId)

    : mKind(aKind) {

  static_assert(std::is_trivially_copyable_v<BaseProfilerThreadId>);

  static_assert(sizeof(aThreadId) <= sizeof(mStorage));

  memcpy(mStorage, &aThreadId, sizeof(aThreadId));

}



const char* ProfileBufferEntry::GetString() const {

  const char* result;

  memcpy(&result, mStorage, sizeof(result));
@@ -99,6 +107,13 @@ uint64_t ProfileBufferEntry::GetUint64() const { 
  return result;

}



BaseProfilerThreadId ProfileBufferEntry::GetThreadId() const {

  BaseProfilerThreadId result;

  static_assert(std::is_trivially_copyable_v<BaseProfilerThreadId>);

  memcpy(&result, mStorage, sizeof(result));

  return result;

}



void ProfileBufferEntry::CopyCharsInto(char (&aOutArray)[kNumChars]) const {

  memcpy(aOutArray, mStorage, kNumChars);

}
@@ -582,8 +597,7 @@ BaseProfilerThreadId ProfileBuffer::StreamSamplesToJSON( 
      // must be a ThreadId entry.

      MOZ_ASSERT(e.Get().IsThreadId());



      BaseProfilerThreadId threadId =

          BaseProfilerThreadId::FromNumber(e.Get().GetInt());

      BaseProfilerThreadId threadId = e.Get().GetThreadId();

      e.Next();



      // Ignore samples that are for the wrong thread.
@@ -1209,8 +1223,7 @@ bool ProfileBuffer::DuplicateLastSample(BaseProfilerThreadId aThreadId, 
    }



    MOZ_RELEASE_ASSERT(e.Has() && e.Get().IsThreadId() &&

                       BaseProfilerThreadId::FromNumber(e.Get().GetInt()) ==

                           aThreadId);

                       e.Get().GetThreadId() == aThreadId);



    e.Next();

mozglue/baseprofiler/core/ProfileBufferEntry.h

+2 −0
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ class ProfileBufferEntry { 
  ProfileBufferEntry(Kind aKind, int64_t aInt64);

  ProfileBufferEntry(Kind aKind, uint64_t aUint64);

  ProfileBufferEntry(Kind aKind, int aInt);

  ProfileBufferEntry(Kind aKind, BaseProfilerThreadId aThreadId);



 public:

#define CTOR(KIND, TYPE, SIZE)                   \
@@ -74,6 +75,7 @@ class ProfileBufferEntry { 
  int GetInt() const;

  int64_t GetInt64() const;

  uint64_t GetUint64() const;

  BaseProfilerThreadId GetThreadId() const;

  void CopyCharsInto(char (&aOutArray)[kNumChars]) const;

};

mozglue/baseprofiler/core/ProfiledThreadData.cpp

+4 −0
Original line number Diff line number Diff line
@@ -175,6 +175,10 @@ BaseProfilerThreadId StreamSamplesAndMarkers( 
  }

  aWriter.EndObject();



  // Tech note: If `ToNumber()` returns a uint64_t, the conversion to int64_t is

  // "implementation-defined" before C++20. This is acceptable here, because

  // this is a one-way conversion to a unique identifier that's used to visually

  // separate data by thread on the front-end.

  aWriter.IntProperty(

      "pid", static_cast<int64_t>(profiler_current_process_id().ToNumber()));

  aWriter.IntProperty("tid",

mozglue/baseprofiler/core/platform.cpp

+6 −4
Original line number Diff line number Diff line
@@ -3365,8 +3365,9 @@ ProfilingStack* profiler_register_thread(const char* aName, 


  if (RegisteredThread* thread = FindCurrentThreadRegisteredThread(lock);

      thread) {

    LOG("profiler_register_thread(%s) - thread %d already registered as %s",

        aName, int(profiler_current_thread_id().ToNumber()),

    LOG("profiler_register_thread(%s) - thread %" PRIu64

        " already registered as %s",

        aName, uint64_t(profiler_current_thread_id().ToNumber()),

        thread->Info()->Name());

    // TODO: Use new name. This is currently not possible because the

    // RegisteredThread's ThreadInfo cannot be changed.
@@ -3416,8 +3417,9 @@ void profiler_unregister_thread() { 
    // registeredThread object.

    CorePS::RemoveRegisteredThread(lock, registeredThread);

  } else {

    LOG("profiler_unregister_thread() - thread %d already unregistered",

        (profiler_current_thread_id().ToNumber()));

    LOG("profiler_unregister_thread() - thread %" PRIu64

        " already unregistered",

        uint64_t(profiler_current_thread_id().ToNumber()));

    // We cannot record a marker on this thread because it was already

    // unregistered. Send it to the main thread (unless this *is* already the

    // main thread, which has been unregistered); this may be useful to catch