From d926fdb58689a19b8cf946a05a8d0f967c9c3927 Mon Sep 17 00:00:00 2001
From: Gerald Squelart <gsquelart@mozilla.com>
Date: Thu, 9 Sep 2021 09:37:29 +0000
Subject: [PATCH] Bug 1728993 - Handle same-sample case when there are no
 stacks - r=florian

A scenario for a thread may be that the first entry has an empty sample (usually if stack-sampling is turned off) and no running times (first-ever sample, or "cpu" is off), so it's not output, and there is no corresponding stack table entry.
Then there may be a "same sample" entry.
- If there are no running times, nothing should be output.
- If there are running times, an empty stack should be output, but for that we need to ensure that the root frame is present in the stack table entry, because that represents an empty stack.

Differential Revision: https://phabricator.services.mozilla.com/D124612
---
 tools/profiler/core/ProfileBufferEntry.cpp | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/profiler/core/ProfileBufferEntry.cpp b/tools/profiler/core/ProfileBufferEntry.cpp
index 669d0bd5be836..2b5be73903410 100644
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -538,6 +538,7 @@ void JITFrameInfo::AddInfoForRange(
 
 struct ProfileSample {
   uint32_t mStack = 0;
+  bool mStackIsEmpty = false;
   double mTime = 0.0;
   Maybe<double> mResponsiveness;
   RunningTimes mRunningTimes;
@@ -1010,15 +1011,19 @@ ProfilerThreadId ProfileBuffer::StreamSamplesToJSON(
           }
         }
 
-        if (numFrames == 0 && aRunningTimes.IsEmpty()) {
+        // Even if this stack is considered empty, it contains the root frame,
+        // which needs to be in the JSON output because following "same samples"
+        // may refer to it when reusing this sample.mStack.
+        sample.mStack = aUniqueStacks.GetOrAddStackIndex(stack);
+        sample.mStackIsEmpty = (numFrames == 0);
+
+        if (sample.mStackIsEmpty && aRunningTimes.IsEmpty()) {
           // It is possible to have empty stacks if native stackwalking is
           // disabled. Skip samples with empty stacks, unless we have useful
           // running times.
           return;
         }
 
-        sample.mStack = aUniqueStacks.GetOrAddStackIndex(stack);
-
         if (unresponsiveDuration.isSome()) {
           sample.mResponsiveness = unresponsiveDuration;
         }
@@ -1107,6 +1112,7 @@ ProfilerThreadId ProfileBuffer::StreamSamplesToJSON(
         }
 
         // Keep the same `mStack` as previously output.
+        // Note that it may be empty, this is checked below before writing it.
         (void)sample.mStack;
 
         sample.mTime = e.Get().GetDouble();
@@ -1138,6 +1144,11 @@ ProfilerThreadId ProfileBuffer::StreamSamplesToJSON(
           }
 
           if (kind == ProfileBufferEntry::Kind::SameSample) {
+            if (sample.mStackIsEmpty && sample.mRunningTimes.IsEmpty()) {
+              // Skip samples with empty stacks, unless we have useful running
+              // times.
+              break;
+            }
             WriteSample(aWriter, sample);
             break;
           }
-- 
GitLab