Bug 1672310 - Output marker backtraces from other threads - r=gregtatum

The new Markers 2.0 code had missed one detail:
Backtraces in markers were serialized as just the `ProfileChunkedBuffer`, which doesn't expose the original thread id like `ProfilerBacktrace` did.
Then when outputting the profile, the marker code would use the marker's thread id (where the marker should be displayed in the frontend, which *could* be different from where the backtrace came) to deserialize and stream the attached marker, and a special check in the streaming code meant that the mismatched id would ignore the stored sample, and the displayed marker would show no stack.

With this patch, when streaming stacks from markers, the given thread id is 0 (an impossible thread id), which indicates that whatever sample is present should be streamed.
`ProfilerBacktrace` doesn't need to store the thread id anymore.
This solves the above problem.

As a bonus, the streaming code now reports the original thread of the sample(s) it found. This could be used in the future, to better show in the frontend that some stacks may come from other threads.

Differential Revision: https://phabricator.services.mozilla.com/D94264

mozglue/baseprofiler/core/ProfileBuffer.h

@@ -51,10 +51,11 @@ class ProfileBuffer final {
   // Stream JSON for samples in the buffer to aWriter, using the supplied
   // UniqueStacks object.
   // Only streams samples for the given thread ID and which were taken at or
-  // after aSinceTime.
-  void StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
-                           double aSinceTime,
-                           UniqueStacks& aUniqueStacks) const;
+  // after aSinceTime. If ID is 0, ignore the stored thread ID; this should only
+  // be used when the buffer contains only one sample.
+  // Return the thread ID of the streamed sample(s), or 0.
+  int StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
+                          double aSinceTime, UniqueStacks& aUniqueStacks) const;
 
   void StreamMarkersToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
                            const TimeStamp& aProcessStartTime,

mozglue/baseprofiler/core/ProfileBufferEntry.cpp

@@ -571,16 +571,18 @@ class EntryGetter {
     continue;                                              \
   }
 
-void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
-                                        int aThreadId, double aSinceTime,
-                                        UniqueStacks& aUniqueStacks) const {
+int ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
+                                       int aThreadId, double aSinceTime,
+                                       UniqueStacks& aUniqueStacks) const {
   UniquePtr<char[]> dynStrBuf = MakeUnique<char[]>(kMaxFrameKeyLength);
 
-  mEntries.Read([&](ProfileChunkedBuffer::Reader* aReader) {
+  return mEntries.Read([&](ProfileChunkedBuffer::Reader* aReader) {
     MOZ_ASSERT(aReader,
                "ProfileChunkedBuffer cannot be out-of-session when sampler is "
                "running");
 
+    int processedThreadId = 0;
+
     EntryGetter e(*aReader);
 
     for (;;) {
@@ -606,20 +608,21 @@ void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
         break;
       }
 
-      if (e.Get().IsThreadId()) {
-        int threadId = e.Get().GetInt();
-        e.Next();
+      // Due to the skip_to_next_sample block above, if we have an entry here it
+      // must be a ThreadId entry.
+      MOZ_ASSERT(e.Get().IsThreadId());
 
-        // Ignore samples that are for the wrong thread.
-        if (threadId != aThreadId) {
-          continue;
-        }
-      } else {
-        // Due to the skip_to_next_sample block above, if we have an entry here
-        // it must be a ThreadId entry.
-        MOZ_CRASH();
+      int threadId = e.Get().GetInt();
+      e.Next();
+
+      // Ignore samples that are for the wrong thread.
+      if (threadId != aThreadId && aThreadId != 0) {
+        continue;
       }
 
+      MOZ_ASSERT(aThreadId != 0 || processedThreadId == 0,
+                 "aThreadId==0 should only be used with 1-sample buffer");
+
       ProfileSample sample;
 
       if (e.Has() && e.Get().IsTime()) {
@@ -794,7 +797,11 @@ void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
       }
 
       WriteSample(aWriter, sample);
+
+      processedThreadId = threadId;
     }
+
+    return processedThreadId;
   });
 }
 
@@ -817,7 +824,7 @@ void ProfileBuffer::StreamMarkersToJSON(SpliceableJSONWriter& aWriter,
                   aWriter, aName.String().c_str());
             },
             [&](ProfileChunkedBuffer& aChunkedBuffer) {
-              ProfilerBacktrace backtrace("", aThreadId, &aChunkedBuffer);
+              ProfilerBacktrace backtrace("", &aChunkedBuffer);
               backtrace.StreamJSON(aWriter, TimeStamp::ProcessCreation(),
                                    aUniqueStacks);
             })) {

mozglue/baseprofiler/core/ProfiledThreadData.cpp

@@ -81,12 +81,14 @@ void ProfiledThreadData::StreamJSON(const ProfileBuffer& aBuffer,
   aWriter.End();
 }
 
-void StreamSamplesAndMarkers(
+int StreamSamplesAndMarkers(
     const char* aName, int aThreadId, const ProfileBuffer& aBuffer,
     SpliceableJSONWriter& aWriter, const std::string& aProcessName,
     const std::string& aETLDplus1, const TimeStamp& aProcessStartTime,
     const TimeStamp& aRegisterTime, const TimeStamp& aUnregisterTime,
     double aSinceTime, UniqueStacks& aUniqueStacks) {
+  int processedThreadId = 0;
+
   aWriter.StringProperty(
       "processType",
       "(unknown)" /* XRE_GeckoProcessTypeToString(XRE_GetProcessType()) */);
@@ -111,10 +113,6 @@ void StreamSamplesAndMarkers(
     aWriter.StringProperty("eTLD+1", aETLDplus1);
   }
 
-  aWriter.IntProperty("tid", static_cast<int64_t>(aThreadId));
-  aWriter.IntProperty("pid",
-                      static_cast<int64_t>(profiler_current_process_id()));
-
   if (aRegisterTime) {
     aWriter.DoubleProperty(
         "registerTime", (aRegisterTime - aProcessStartTime).ToMilliseconds());
@@ -141,8 +139,8 @@ void StreamSamplesAndMarkers(
 
     aWriter.StartArrayProperty("data");
     {
-      aBuffer.StreamSamplesToJSON(aWriter, aThreadId, aSinceTime,
-                                  aUniqueStacks);
+      processedThreadId = aBuffer.StreamSamplesToJSON(
+          aWriter, aThreadId, aSinceTime, aUniqueStacks);
     }
     aWriter.EndArray();
   }
@@ -168,6 +166,14 @@ void StreamSamplesAndMarkers(
     aWriter.EndArray();
   }
   aWriter.EndObject();
+
+  aWriter.IntProperty("pid",
+                      static_cast<int64_t>(profiler_current_process_id()));
+  aWriter.IntProperty(
+      "tid",
+      static_cast<int64_t>(aThreadId != 0 ? aThreadId : processedThreadId));
+
+  return processedThreadId;
 }
 
 }  // namespace baseprofiler

mozglue/baseprofiler/core/ProfiledThreadData.h

@@ -103,7 +103,10 @@ class ProfiledThreadData final {
   TimeStamp mUnregisterTime;
 };
 
-void StreamSamplesAndMarkers(
+// Stream all samples and markers from aBuffer with the given aThreadId (or 0
+// for everything, which is assumed to be a single backtrace sample.)
+// Returns the thread id of the output sample(s), or 0 if none was present.
+int StreamSamplesAndMarkers(
     const char* aName, int aThreadId, const ProfileBuffer& aBuffer,
     SpliceableJSONWriter& aWriter, const std::string& aProcessName,
     const std::string& aETLDplus1, const TimeStamp& aProcessStartTime,

mozglue/baseprofiler/core/ProfilerBacktrace.cpp

@@ -17,11 +17,10 @@ namespace mozilla {
 namespace baseprofiler {
 
 ProfilerBacktrace::ProfilerBacktrace(
-    const char* aName, int aThreadId,
+    const char* aName,
     UniquePtr<ProfileChunkedBuffer> aProfileChunkedBufferStorage,
     UniquePtr<ProfileBuffer> aProfileBufferStorageOrNull /* = nullptr */)
     : mName(aName),
-      mThreadId(aThreadId),
       mOptionalProfileChunkedBufferStorage(
           std::move(aProfileChunkedBufferStorage)),
       mProfileChunkedBuffer(mOptionalProfileChunkedBufferStorage.get()),
@@ -42,11 +41,10 @@ ProfilerBacktrace::ProfilerBacktrace(
 }
 
 ProfilerBacktrace::ProfilerBacktrace(
-    const char* aName, int aThreadId,
+    const char* aName,
     ProfileChunkedBuffer* aExternalProfileChunkedBufferOrNull /* = nullptr */,
     ProfileBuffer* aExternalProfileBufferOrNull /* = nullptr */)
     : mName(aName),
-      mThreadId(aThreadId),
       mProfileChunkedBuffer(aExternalProfileChunkedBufferOrNull),
       mProfileBuffer(aExternalProfileBufferOrNull) {
   if (!mProfileChunkedBuffer) {
@@ -73,28 +71,32 @@ ProfilerBacktrace::ProfilerBacktrace(
 
 ProfilerBacktrace::~ProfilerBacktrace() {}
 
-void ProfilerBacktrace::StreamJSON(SpliceableJSONWriter& aWriter,
-                                   const TimeStamp& aProcessStartTime,
-                                   UniqueStacks& aUniqueStacks) {
+int ProfilerBacktrace::StreamJSON(SpliceableJSONWriter& aWriter,
+                                  const TimeStamp& aProcessStartTime,
+                                  UniqueStacks& aUniqueStacks) {
+  int processedThreadId = 0;
+
   // Unlike ProfiledThreadData::StreamJSON, we don't need to call
   // ProfileBuffer::AddJITInfoForRange because ProfileBuffer does not contain
   // any JitReturnAddr entries. For synchronous samples, JIT frames get expanded
   // at sample time.
   if (mProfileBuffer) {
-    StreamSamplesAndMarkers(mName.c_str(), mThreadId, *mProfileBuffer, aWriter,
-                            "", "", aProcessStartTime,
-                            /* aRegisterTime */ TimeStamp(),
-                            /* aUnregisterTime */ TimeStamp(),
-                            /* aSinceTime */ 0, aUniqueStacks);
+    processedThreadId = StreamSamplesAndMarkers(
+        mName.c_str(), 0, *mProfileBuffer, aWriter, "", "", aProcessStartTime,
+        /* aRegisterTime */ TimeStamp(),
+        /* aUnregisterTime */ TimeStamp(),
+        /* aSinceTime */ 0, aUniqueStacks);
   } else if (mProfileChunkedBuffer) {
     ProfileBuffer profileBuffer(*mProfileChunkedBuffer);
-    StreamSamplesAndMarkers(mName.c_str(), mThreadId, profileBuffer, aWriter,
-                            "", "", aProcessStartTime,
-                            /* aRegisterTime */ TimeStamp(),
-                            /* aUnregisterTime */ TimeStamp(),
-                            /* aSinceTime */ 0, aUniqueStacks);
+    processedThreadId = StreamSamplesAndMarkers(
+        mName.c_str(), 0, profileBuffer, aWriter, "", "", aProcessStartTime,
+        /* aRegisterTime */ TimeStamp(),
+        /* aUnregisterTime */ TimeStamp(),
+        /* aSinceTime */ 0, aUniqueStacks);
   }
   // If there are no buffers, the backtrace is empty and nothing is streamed.
+
+  return processedThreadId;
 }
 
 }  // namespace baseprofiler
@@ -112,10 +114,9 @@ ProfileBufferEntryReader::
   MOZ_ASSERT(
       !profileChunkedBuffer->IsThreadSafe(),
       "ProfilerBacktrace only stores non-thread-safe ProfileChunkedBuffers");
-  int threadId = aER.ReadObject<int>();
   std::string name = aER.ReadObject<std::string>();
   return UniquePtr<baseprofiler::ProfilerBacktrace, Destructor>{
-      new baseprofiler::ProfilerBacktrace(name.c_str(), threadId,
+      new baseprofiler::ProfilerBacktrace(name.c_str(),
                                           std::move(profileChunkedBuffer))};
 };
 

mozglue/baseprofiler/core/ProfilerBacktrace.h

@@ -39,16 +39,16 @@ class ProfilerBacktrace {
   // Take ownership of external buffers and use them to keep, and to stream a
   // backtrace. If a ProfileBuffer is given, its underlying chunked buffer must
   // be provided as well.
-  ProfilerBacktrace(
-      const char* aName, int aThreadId,
+  explicit ProfilerBacktrace(
+      const char* aName,
       UniquePtr<ProfileChunkedBuffer> aProfileChunkedBufferStorage,
       UniquePtr<ProfileBuffer> aProfileBufferStorageOrNull = nullptr);
 
   // Take pointers to external buffers and use them to stream a backtrace.
   // If null, the backtrace is effectively empty.
   // If both are provided, they must already be connected.
-  ProfilerBacktrace(
-      const char* aName, int aThreadId,
+  explicit ProfilerBacktrace(
+      const char* aName,
       ProfileChunkedBuffer* aExternalProfileChunkedBufferOrNull = nullptr,
       ProfileBuffer* aExternalProfileBufferOrNull = nullptr);
 
@@ -66,9 +66,9 @@ class ProfilerBacktrace {
   // That is, markers that contain backtraces should not need their own stack,
   // frame, and string tables. They should instead reuse their parent
   // profile's tables.
-  void StreamJSON(SpliceableJSONWriter& aWriter,
-                  const TimeStamp& aProcessStartTime,
-                  UniqueStacks& aUniqueStacks);
+  int StreamJSON(SpliceableJSONWriter& aWriter,
+                 const TimeStamp& aProcessStartTime,
+                 UniqueStacks& aUniqueStacks);
 
  private:
   // Used to de/serialize a ProfilerBacktrace.
@@ -76,7 +76,6 @@ class ProfilerBacktrace {
   friend ProfileBufferEntryReader::Deserializer<ProfilerBacktrace>;
 
   std::string mName;
-  int mThreadId;
 
   // `ProfileChunkedBuffer` in which `mProfileBuffer` stores its data; must be
   // located before `mProfileBuffer` so that it's destroyed after.
@@ -91,7 +90,7 @@ class ProfilerBacktrace {
 
 }  // namespace baseprofiler
 
-// Format: [ UniquePtr<BlockRingsBuffer> | threadId | name ]
+// Format: [ UniquePtr<BlockRingsBuffer> | name ]
 // Initial len==0 marks a nullptr or empty backtrace.
 template <>
 struct ProfileBufferEntryWriter::Serializer<baseprofiler::ProfilerBacktrace> {
@@ -105,7 +104,7 @@ struct ProfileBufferEntryWriter::Serializer<baseprofiler::ProfilerBacktrace> {
       // Empty buffer.
       return ULEB128Size(0u);
     }
-    return bufferBytes + SumBytes(aBacktrace.mThreadId, aBacktrace.mName);
+    return bufferBytes + SumBytes(aBacktrace.mName);
   }
 
   static void Write(ProfileBufferEntryWriter& aEW,
@@ -117,7 +116,6 @@ struct ProfileBufferEntryWriter::Serializer<baseprofiler::ProfilerBacktrace> {
       return;
     }
     aEW.WriteObject(*aBacktrace.mProfileChunkedBuffer);
-    aEW.WriteObject(aBacktrace.mThreadId);
     aEW.WriteObject(aBacktrace.mName);
   }
 };

mozglue/baseprofiler/core/platform.cpp

@@ -3603,8 +3603,8 @@ UniqueProfilerBacktrace profiler_get_backtrace() {
     return nullptr;
   }
 
-  return UniqueProfilerBacktrace(new ProfilerBacktrace(
-      "SyncProfile", profiler_current_thread_id(), std::move(buffer)));
+  return UniqueProfilerBacktrace(
+      new ProfilerBacktrace("SyncProfile", std::move(buffer)));
 }
 
 void ProfilerBacktraceDestructor::operator()(ProfilerBacktrace* aBacktrace) {

tools/profiler/core/ProfileBuffer.h

@@ -57,14 +57,15 @@ class ProfileBuffer final {
   // Stream JSON for samples in the buffer to aWriter, using the supplied
   // UniqueStacks object.
   // Only streams samples for the given thread ID and which were taken at or
-  // after aSinceTime.
+  // after aSinceTime. If ID is 0, ignore the stored thread ID; this should only
+  // be used when the buffer contains only one sample.
   // aUniqueStacks needs to contain information about any JIT frames that we
   // might encounter in the buffer, before this method is called. In other
   // words, you need to have called AddJITInfoForRange for every range that
   // might contain JIT frame information before calling this method.
-  void StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
-                           double aSinceTime,
-                           UniqueStacks& aUniqueStacks) const;
+  // Return the thread ID of the streamed sample(s), or 0.
+  int StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
+                          double aSinceTime, UniqueStacks& aUniqueStacks) const;
 
   void StreamMarkersToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
                            const mozilla::TimeStamp& aProcessStartTime,

tools/profiler/core/ProfileBufferEntry.cpp

@@ -784,16 +784,18 @@ class EntryGetter {
     continue;                                              \
   }
 
-void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
-                                        int aThreadId, double aSinceTime,
-                                        UniqueStacks& aUniqueStacks) const {
+int ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
+                                       int aThreadId, double aSinceTime,
+                                       UniqueStacks& aUniqueStacks) const {
   UniquePtr<char[]> dynStrBuf = MakeUnique<char[]>(kMaxFrameKeyLength);
 
-  mEntries.Read([&](ProfileChunkedBuffer::Reader* aReader) {
+  return mEntries.Read([&](ProfileChunkedBuffer::Reader* aReader) {
     MOZ_ASSERT(aReader,
                "ProfileChunkedBuffer cannot be out-of-session when sampler is "
                "running");
 
+    int processedThreadId = 0;
+
     EntryGetter e(*aReader);
 
     for (;;) {
@@ -819,20 +821,21 @@ void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
         break;
       }
 
-      if (e.Get().IsThreadId()) {
-        int threadId = e.Get().GetInt();
-        e.Next();
+      // Due to the skip_to_next_sample block above, if we have an entry here it
+      // must be a ThreadId entry.
+      MOZ_ASSERT(e.Get().IsThreadId());
 
-        // Ignore samples that are for the wrong thread.
-        if (threadId != aThreadId) {
-          continue;
-        }
-      } else {
-        // Due to the skip_to_next_sample block above, if we have an entry here
-        // it must be a ThreadId entry.
-        MOZ_CRASH();
+      int threadId = e.Get().GetInt();
+      e.Next();
+
+      // Ignore samples that are for the wrong thread.
+      if (threadId != aThreadId && aThreadId != 0) {
+        continue;
       }
 
+      MOZ_ASSERT(aThreadId != 0 || processedThreadId == 0,
+                 "aThreadId==0 should only be used with 1-sample buffer");
+
       ProfileSample sample;
 
       auto ReadStack = [&](EntryGetter& e, uint64_t entryPosition,
@@ -1000,6 +1003,8 @@ void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
         }
 
         WriteSample(aWriter, sample);
+
+        processedThreadId = threadId;
       };  // End of `ReadStack(EntryGetter&)` lambda.
 
       if (e.Has() && e.Get().IsTime()) {
@@ -1069,6 +1074,8 @@ void ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter,
         ERROR_AND_CONTINUE("expected a Time entry");
       }
     }
+
+    return processedThreadId;
   });
 }
 
@@ -1240,8 +1247,7 @@ void ProfileBuffer::StreamMarkersToJSON(SpliceableJSONWriter& aWriter,
                           aWriter, aName.String().c_str());
                     },
                     [&](ProfileChunkedBuffer& aChunkedBuffer) {
-                      ProfilerBacktrace backtrace("", aThreadId,
-                                                  &aChunkedBuffer);
+                      ProfilerBacktrace backtrace("", &aChunkedBuffer);
                       backtrace.StreamJSON(aWriter, aProcessStartTime,
                                            aUniqueStacks);
                     })) {

tools/profiler/core/ProfiledThreadData.cpp

@@ -196,15 +196,17 @@ void ProfiledThreadData::StreamTraceLoggerJSON(
   aWriter.EndObject();
 }
 
-void StreamSamplesAndMarkers(const char* aName, int aThreadId,
-                             const ProfileBuffer& aBuffer,
-                             SpliceableJSONWriter& aWriter,
-                             const nsACString& aProcessName,
-                             const nsACString& aETLDplus1,
-                             const mozilla::TimeStamp& aProcessStartTime,
-                             const mozilla::TimeStamp& aRegisterTime,
-                             const mozilla::TimeStamp& aUnregisterTime,
-                             double aSinceTime, UniqueStacks& aUniqueStacks) {
+int StreamSamplesAndMarkers(const char* aName, int aThreadId,
+                            const ProfileBuffer& aBuffer,
+                            SpliceableJSONWriter& aWriter,
+                            const nsACString& aProcessName,
+                            const nsACString& aETLDplus1,
+                            const mozilla::TimeStamp& aProcessStartTime,
+                            const mozilla::TimeStamp& aRegisterTime,
+                            const mozilla::TimeStamp& aUnregisterTime,
+                            double aSinceTime, UniqueStacks& aUniqueStacks) {
+  int processedThreadId = 0;
+
   aWriter.StringProperty("processType",
                          MakeStringSpan(XRE_GetProcessTypeString()));
 
@@ -220,10 +222,6 @@ void StreamSamplesAndMarkers(const char* aName, int aThreadId,
     aWriter.StringProperty("eTLD+1", aETLDplus1);
   }
 
-  aWriter.IntProperty("tid", static_cast<int64_t>(aThreadId));
-  aWriter.IntProperty("pid",
-                      static_cast<int64_t>(profiler_current_process_id()));
-
   if (aRegisterTime) {
     aWriter.DoubleProperty(
         "registerTime", (aRegisterTime - aProcessStartTime).ToMilliseconds());
@@ -250,8 +248,8 @@ void StreamSamplesAndMarkers(const char* aName, int aThreadId,
 
     aWriter.StartArrayProperty("data");
     {
-      aBuffer.StreamSamplesToJSON(aWriter, aThreadId, aSinceTime,
-                                  aUniqueStacks);
+      processedThreadId = aBuffer.StreamSamplesToJSON(
+          aWriter, aThreadId, aSinceTime, aUniqueStacks);
     }
     aWriter.EndArray();
   }
@@ -277,6 +275,14 @@ void StreamSamplesAndMarkers(const char* aName, int aThreadId,
     aWriter.EndArray();
   }
   aWriter.EndObject();
+
+  aWriter.IntProperty("pid",
+                      static_cast<int64_t>(profiler_current_process_id()));
+  aWriter.IntProperty(
+      "tid",
+      static_cast<int64_t>(aThreadId != 0 ? aThreadId : processedThreadId));
+
+  return processedThreadId;
 }
 
 void ProfiledThreadData::NotifyAboutToLoseJSContext(

tools/profiler/core/ProfiledThreadData.h

@@ -116,14 +116,17 @@ class ProfiledThreadData final {
   mozilla::TimeStamp mUnregisterTime;
 };
 
-void StreamSamplesAndMarkers(const char* aName, int aThreadId,
-                             const ProfileBuffer& aBuffer,
-                             SpliceableJSONWriter& aWriter,
-                             const nsACString& aProcessName,
-                             const nsACString& aETLDplus1,
-                             const mozilla::TimeStamp& aProcessStartTime,
-                             const mozilla::TimeStamp& aRegisterTime,
-                             const mozilla::TimeStamp& aUnregisterTime,
-                             double aSinceTime, UniqueStacks& aUniqueStacks);
+// Stream all samples and markers from aBuffer with the given aThreadId (or 0
+// for everything, which is assumed to be a single backtrace sample.)
+// Returns the thread id of the output sample(s), or 0 if none was present.
+int StreamSamplesAndMarkers(const char* aName, int aThreadId,
+                            const ProfileBuffer& aBuffer,
+                            SpliceableJSONWriter& aWriter,
+                            const nsACString& aProcessName,
+                            const nsACString& aETLDplus1,
+                            const mozilla::TimeStamp& aProcessStartTime,
+                            const mozilla::TimeStamp& aRegisterTime,
+                            const mozilla::TimeStamp& aUnregisterTime,
+                            double aSinceTime, UniqueStacks& aUniqueStacks);
 
 #endif  // ProfiledThreadData_h

tools/profiler/core/ProfilerBacktrace.cpp

@@ -13,13 +13,12 @@
 #include "mozilla/ProfileJSONWriter.h"
 
 ProfilerBacktrace::ProfilerBacktrace(
-    const char* aName, int aThreadId,
+    const char* aName,
     mozilla::UniquePtr<mozilla::ProfileChunkedBuffer>
         aProfileChunkedBufferStorage,
     mozilla::UniquePtr<ProfileBuffer>
         aProfileBufferStorageOrNull /* = nullptr */)
     : mName(aName),
-      mThreadId(aThreadId),
       mOptionalProfileChunkedBufferStorage(
           std::move(aProfileChunkedBufferStorage)),
       mProfileChunkedBuffer(mOptionalProfileChunkedBufferStorage.get()),
@@ -41,11 +40,10 @@ ProfilerBacktrace::ProfilerBacktrace(
 }
 
 ProfilerBacktrace::ProfilerBacktrace(
-    const char* aName, int aThreadId,
+    const char* aName,
     mozilla::ProfileChunkedBuffer* aExternalProfileChunkedBuffer,
     ProfileBuffer* aExternalProfileBuffer)
     : mName(aName),
-      mThreadId(aThreadId),
       mProfileChunkedBuffer(aExternalProfileChunkedBuffer),
       mProfileBuffer(aExternalProfileBuffer) {
   MOZ_COUNT_CTOR(ProfilerBacktrace);
@@ -73,26 +71,32 @@ ProfilerBacktrace::ProfilerBacktrace(
 
 ProfilerBacktrace::~ProfilerBacktrace() { MOZ_COUNT_DTOR(ProfilerBacktrace); }
 
-void ProfilerBacktrace::StreamJSON(SpliceableJSONWriter& aWriter,
-                                   const mozilla::TimeStamp& aProcessStartTime,
-                                   UniqueStacks& aUniqueStacks) {
+int ProfilerBacktrace::StreamJSON(SpliceableJSONWriter& aWriter,
+                                  const mozilla::TimeStamp& aProcessStartTime,
+                                  UniqueStacks& aUniqueStacks) {
+  int processedThreadId = 0;
+
   // Unlike ProfiledThreadData::StreamJSON, we don't need to call
   // ProfileBuffer::AddJITInfoForRange because ProfileBuffer does not contain
   // any JitReturnAddr entries. For synchronous samples, JIT frames get expanded
   // at sample time.
   if (mProfileBuffer) {
-    StreamSamplesAndMarkers(mName.c_str(), mThreadId, *mProfileBuffer, aWriter,
-                            ""_ns, ""_ns, aProcessStartTime,
-                            /* aRegisterTime */ mozilla::TimeStamp(),
-                            /* aUnregisterTime */ mozilla::TimeStamp(),
-                            /* aSinceTime */ 0, aUniqueStacks);
+    processedThreadId =
+        StreamSamplesAndMarkers(mName.c_str(), 0, *mProfileBuffer, aWriter,
+                                ""_ns, ""_ns, aProcessStartTime,
+                                /* aRegisterTime */ mozilla::TimeStamp(),
+                                /* aUnregisterTime */ mozilla::TimeStamp(),
+                                /* aSinceTime */ 0, aUniqueStacks);
   } else if (mProfileChunkedBuffer) {
     ProfileBuffer profileBuffer(*mProfileChunkedBuffer);
-    StreamSamplesAndMarkers(mName.c_str(), mThreadId, profileBuffer, aWriter,
-                            ""_ns, ""_ns, aProcessStartTime,
-                            /* aRegisterTime */ mozilla::TimeStamp(),
-                            /* aUnregisterTime */ mozilla::TimeStamp(),
-                            /* aSinceTime */ 0, aUniqueStacks);
+    processedThreadId =
+        StreamSamplesAndMarkers(mName.c_str(), 0, profileBuffer, aWriter, ""_ns,
+                                ""_ns, aProcessStartTime,
+                                /* aRegisterTime */ mozilla::TimeStamp(),
+                                /* aUnregisterTime */ mozilla::TimeStamp(),
+                                /* aSinceTime */ 0, aUniqueStacks);
   }
   // If there are no buffers, the backtrace is empty and nothing is streamed.
+
+  return processedThreadId;
 }

tools/profiler/core/ProfilerBacktrace.h

@@ -43,8 +43,8 @@ class ProfilerBacktrace {
   // Take ownership of external buffers and use them to keep, and to stream a
   // backtrace. If a ProfileBuffer is given, its underlying chunked buffer must
   // be provided as well.
-  ProfilerBacktrace(
-      const char* aName, int aThreadId,
+  explicit ProfilerBacktrace(
+      const char* aName,
       mozilla::UniquePtr<mozilla::ProfileChunkedBuffer>
           aProfileChunkedBufferStorage,
       mozilla::UniquePtr<ProfileBuffer> aProfileBufferStorageOrNull = nullptr);
@@ -52,10 +52,11 @@ class ProfilerBacktrace {
   // Take pointers to external buffers and use them to stream a backtrace.
   // If null, the backtrace is effectively empty.
   // If both are provided, they must already be connected.
-  ProfilerBacktrace(const char* aName, int aThreadId,
-                    mozilla::ProfileChunkedBuffer*
-                        aExternalProfileChunkedBufferOrNull = nullptr,
-                    ProfileBuffer* aExternalProfileBufferOrNull = nullptr);
+  explicit ProfilerBacktrace(
+      const char* aName,
+      mozilla::ProfileChunkedBuffer* aExternalProfileChunkedBufferOrNull =
+          nullptr,
+      ProfileBuffer* aExternalProfileBufferOrNull = nullptr);
 
   ~ProfilerBacktrace();
 
@@ -72,9 +73,9 @@ class ProfilerBacktrace {
   // That is, markers that contain backtraces should not need their own stack,
   // frame, and string tables. They should instead reuse their parent
   // profile's tables.
-  void StreamJSON(mozilla::baseprofiler::SpliceableJSONWriter& aWriter,
-                  const mozilla::TimeStamp& aProcessStartTime,
-                  UniqueStacks& aUniqueStacks);
+  int StreamJSON(mozilla::baseprofiler::SpliceableJSONWriter& aWriter,
+                 const mozilla::TimeStamp& aProcessStartTime,
+                 UniqueStacks& aUniqueStacks);
 
  private:
   // Used to serialize a ProfilerBacktrace.
@@ -84,7 +85,6 @@ class ProfilerBacktrace {
       ProfilerBacktrace>;
 
   std::string mName;
-  int mThreadId;
 
   // `ProfileChunkedBuffer` in which `mProfileBuffer` stores its data; must be
   // located before `mProfileBuffer` so that it's destroyed after.
@@ -100,7 +100,7 @@ class ProfilerBacktrace {
 
 namespace mozilla {
 
-// Format: [ UniquePtr<BlockRingsBuffer> | threadId | name ]
+// Format: [ UniquePtr<BlockRingsBuffer> | name ]
 // Initial len==0 marks a nullptr or empty backtrace.
 template <>
 struct mozilla::ProfileBufferEntryWriter::Serializer<ProfilerBacktrace> {
@@ -114,7 +114,7 @@ struct mozilla::ProfileBufferEntryWriter::Serializer<ProfilerBacktrace> {
       // Empty buffer.
       return ULEB128Size(0u);
     }
-    return bufferBytes + SumBytes(aBacktrace.mThreadId, aBacktrace.mName);
+    return bufferBytes + SumBytes(aBacktrace.mName);
   }
 
   static void Write(mozilla::ProfileBufferEntryWriter& aEW,
@@ -126,7 +126,6 @@ struct mozilla::ProfileBufferEntryWriter::Serializer<ProfilerBacktrace> {
       return;
     }
     aEW.WriteObject(*aBacktrace.mProfileChunkedBuffer);
-    aEW.WriteObject(aBacktrace.mThreadId);
     aEW.WriteObject(aBacktrace.mName);
   }
 };
@@ -174,10 +173,9 @@ struct mozilla::ProfileBufferEntryReader::Deserializer<
     MOZ_ASSERT(
         !profileChunkedBuffer->IsThreadSafe(),
         "ProfilerBacktrace only stores non-thread-safe ProfileChunkedBuffers");
-    int threadId = aER.ReadObject<int>();
     std::string name = aER.ReadObject<std::string>();
-    return UniquePtr<ProfilerBacktrace, Destructor>{new ProfilerBacktrace(
-        name.c_str(), threadId, std::move(profileChunkedBuffer))};
+    return UniquePtr<ProfilerBacktrace, Destructor>{
+        new ProfilerBacktrace(name.c_str(), std::move(profileChunkedBuffer))};
   }
 };
 

tools/profiler/core/platform.cpp

@@ -5408,8 +5408,8 @@ UniqueProfilerBacktrace profiler_get_backtrace() {
     return nullptr;
   }
 
-  return UniqueProfilerBacktrace(new ProfilerBacktrace(
-      "SyncProfile", profiler_current_thread_id(), std::move(buffer)));
+  return UniqueProfilerBacktrace(
+      new ProfilerBacktrace("SyncProfile", std::move(buffer)));
 }
 
 void ProfilerBacktraceDestructor::operator()(ProfilerBacktrace* aBacktrace) {

tools/profiler/tests/gtest/GeckoProfiler.cpp

@@ -877,6 +877,45 @@ TEST(GeckoProfiler, Markers)
        mozilla::ipc::MessageDirection::eSending,
        mozilla::ipc::MessagePhase::Endpoint, false, ts1));
 
+  MOZ_RELEASE_ASSERT(profiler_add_marker(
+      "Text in main thread with stack", geckoprofiler::category::OTHER,
+      MarkerStack::Capture(), geckoprofiler::markers::Text{}, ""));
+  MOZ_RELEASE_ASSERT(profiler_add_marker(
+      "Text from main thread with stack", geckoprofiler::category::OTHER,
+      MarkerOptions(MarkerThreadId::MainThread(), MarkerStack::Capture()),
+      geckoprofiler::markers::Text{}, ""));
+
+  std::thread registeredThread([]() {
+    AUTO_PROFILER_REGISTER_THREAD("Marker test sub-thread");
+    // Marker in non-profiled thread won't be stored.
+    MOZ_RELEASE_ASSERT(profiler_add_marker(
+        "Text in registered thread with stack", geckoprofiler::category::OTHER,
+        MarkerStack::Capture(), geckoprofiler::markers::Text{}, ""));
+    // Marker will be stored in main thread, with stack from registered thread.
+    MOZ_RELEASE_ASSERT(profiler_add_marker(
+        "Text from registered thread with stack",
+        geckoprofiler::category::OTHER,
+        MarkerOptions(MarkerThreadId::MainThread(), MarkerStack::Capture()),
+        geckoprofiler::markers::Text{}, ""));
+  });
+  registeredThread.join();
+
+  std::thread unregisteredThread([]() {
+    // Marker in unregistered thread won't be stored.
+    MOZ_RELEASE_ASSERT(profiler_add_marker(
+        "Text in unregistered thread with stack",
+        geckoprofiler::category::OTHER, MarkerStack::Capture(),
+        geckoprofiler::markers::Text{}, ""));
+    // Marker will be stored in main thread, but stack cannot be captured in an
+    // unregistered thread.
+    MOZ_RELEASE_ASSERT(profiler_add_marker(
+        "Text from unregistered thread with stack",
+        geckoprofiler::category::OTHER,
+        MarkerOptions(MarkerThreadId::MainThread(), MarkerStack::Capture()),
+        geckoprofiler::markers::Text{}, ""));
+  });
+  unregisteredThread.join();
+
   MOZ_RELEASE_ASSERT(
       profiler_add_marker("Tracing", geckoprofiler::category::OTHER, {},
                           geckoprofiler::markers::Tracing{}, "category"));
@@ -978,6 +1017,10 @@ TEST(GeckoProfiler, Markers)
     S_UserTimingMarkerPayload_measure,
     S_VsyncMarkerPayload,
     S_IPCMarkerPayload,
+    S_TextWithStack,
+    S_TextToMTWithStack,
+    S_RegThread_TextToMTWithStack,
+    S_UnregThread_TextToMTWithStack,
 
     S_LAST,
   } state = State(0);
@@ -1546,6 +1589,45 @@ TEST(GeckoProfiler, Markers)
                 EXPECT_EQ_JSON(payload["direction"], String, "sending");
                 EXPECT_EQ_JSON(payload["phase"], String, "endpoint");
                 EXPECT_EQ_JSON(payload["sync"], Bool, false);
+
+              } else if (nameString == "Text in main thread with stack") {
+                EXPECT_EQ(state, S_TextWithStack);
+                state = State(S_TextWithStack + 1);
+                EXPECT_EQ(typeString, "Text");
+                EXPECT_FALSE(payload["stack"].isNull());
+                EXPECT_EQ_JSON(payload["name"], String, "");
+
+              } else if (nameString == "Text from main thread with stack") {
+                EXPECT_EQ(state, S_TextToMTWithStack);
+                state = State(S_TextToMTWithStack + 1);
+                EXPECT_EQ(typeString, "Text");
+                EXPECT_FALSE(payload["stack"].isNull());
+                EXPECT_EQ_JSON(payload["name"], String, "");
+
+              } else if (nameString == "Text in registered thread with stack") {
+                ADD_FAILURE()
+                    << "Unexpected 'Text in registered thread with stack'";
+
+              } else if (nameString ==
+                         "Text from registered thread with stack") {
+                EXPECT_EQ(state, S_RegThread_TextToMTWithStack);
+                state = State(S_RegThread_TextToMTWithStack + 1);
+                EXPECT_EQ(typeString, "Text");
+                EXPECT_FALSE(payload["stack"].isNull());
+                EXPECT_EQ_JSON(payload["name"], String, "");
+
+              } else if (nameString ==
+                         "Text in unregistered thread with stack") {
+                ADD_FAILURE()
+                    << "Unexpected 'Text in unregistered thread with stack'";
+
+              } else if (nameString ==
+                         "Text from unregistered thread with stack") {
+                EXPECT_EQ(state, S_UnregThread_TextToMTWithStack);
+                state = State(S_UnregThread_TextToMTWithStack + 1);
+                EXPECT_EQ(typeString, "Text");
+                EXPECT_TRUE(payload["stack"].isNull());
+                EXPECT_EQ_JSON(payload["name"], String, "");
               }
             }  // marker with payload
           }    // for (marker:data)