bug 785922: Emit column numbers for JS frames and functions in the gecko profiler r=sfink,mstange

Add support for column numbers when profiling JS stack frames and functions.  This will help debug minified scripts when inspecting performance profiles.  The column information will be emitted as a new column property that is part of the frameTable in the profile output, and will also be appended in the descriptive profiler string.

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

--HG--
extra : moz-landing-system : lando

js/src/jit/JitcodeMap.cpp

@@ -324,29 +324,31 @@ JitcodeGlobalEntry::createScriptString(JSContext* cx, JSScript* script, size_t*
     const char* filenameStr = script->filename() ? script->filename() : "(null)";
     size_t filenameLength = strlen(filenameStr);
 
-    // Calculate lineno length
-    bool hasLineno = false;
-    size_t linenoLength = 0;
-    char linenoStr[15];
+    // Calculate line + column length
+    bool hasLineAndColumn = false;
+    size_t lineAndColumnLength = 0;
+    char lineAndColumnStr[30];
     if (hasName || (script->functionNonDelazifying() || script->isForEval())) {
-        linenoLength = SprintfLiteral(linenoStr, "%u", script->lineno());
-        hasLineno = true;
+        lineAndColumnLength =
+            SprintfLiteral(lineAndColumnStr, "%u:%u",
+                           script->lineno(), script->column());
+        hasLineAndColumn = true;
     }
 
     // Full profile string for scripts with functions is:
-    //      FuncName (FileName:Lineno)
+    //      FuncName (FileName:Lineno:Column)
     // Full profile string for scripts without functions is:
-    //      FileName:Lineno
-    // Full profile string for scripts without functions and without linenos is:
+    //      FileName:Lineno:Column
+    // Full profile string for scripts without functions and without lines is:
     //      FileName
 
     // Calculate full string length.
     size_t fullLength = 0;
     if (hasName) {
-        MOZ_ASSERT(hasLineno);
-        fullLength = nameLength + 2 + filenameLength + 1 + linenoLength + 1;
-    } else if (hasLineno) {
-        fullLength = filenameLength + 1 + linenoLength;
+        MOZ_ASSERT(hasLineAndColumn);
+        fullLength = nameLength + 2 + filenameLength + 1 + lineAndColumnLength + 1;
+    } else if (hasLineAndColumn) {
+        fullLength = filenameLength + 1 + lineAndColumnLength;
     } else {
         fullLength = filenameLength;
     }
@@ -370,11 +372,11 @@ JitcodeGlobalEntry::createScriptString(JSContext* cx, JSScript* script, size_t*
     memcpy(str + cur, filenameStr, filenameLength);
     cur += filenameLength;
 
-    // Fill lineno chars.
-    if (hasLineno) {
+    // Fill line + column chars.
+    if (hasLineAndColumn) {
         str[cur++] = ':';
-        memcpy(str + cur, linenoStr, linenoLength);
-        cur += linenoLength;
+        memcpy(str + cur, lineAndColumnStr, lineAndColumnLength);
+        cur += lineAndColumnLength;
     }
 
     // Terminal ')' if necessary.

js/src/vm/GeckoProfiler.cpp

@@ -275,12 +275,17 @@ GeckoProfilerRuntime::allocProfileString(JSScript* script, JSFunction* maybeFun)
     size_t lenFilename = strlen(filename);
 
     // Get the line number and its length as a string.
-    uint64_t lineno = script->lineno();
+    uint32_t lineno = script->lineno();
     size_t lenLineno = 1;
-    for (uint64_t i = lineno; i /= 10; lenLineno++);
+    for (uint32_t i = lineno; i /= 10; lenLineno++);
+
+    // Get the column number and its length as a string.
+    uint32_t column = script->column();
+    size_t lenColumn = 1;
+    for (uint32_t i = column; i /= 10; lenColumn++);
 
     // Determine the required buffer size.
-    size_t len = lenFilename + lenLineno + 1; // +1 for the ":" separating them.
+    size_t len = lenFilename + 1 + lenLineno + 1 + lenColumn; // +1 for each separator colon, ":".
     if (atom) {
         len += JS::GetDeflatedUTF8StringLength(atom) + 3; // +3 for the " (" and ")" it adds.
     }
@@ -297,9 +302,11 @@ GeckoProfilerRuntime::allocProfileString(JSScript* script, JSFunction* maybeFun)
         if (!atomStr)
             return nullptr;
 
-        ret = snprintf(cstr.get(), len + 1, "%s (%s:%" PRIu64 ")", atomStr.get(), filename, lineno);
+        ret = snprintf(cstr.get(), len + 1, "%s (%s:%" PRIu32 ":%" PRIu32 ")",
+                atomStr.get(), filename, lineno, column);
     } else {
-        ret = snprintf(cstr.get(), len + 1, "%s:%" PRIu64, filename, lineno);
+        ret = snprintf(cstr.get(), len + 1, "%s:%" PRIu32 ":%" PRIu32,
+                filename, lineno, column);
     }
 
     MOZ_ASSERT(ret == len, "Computed length should match actual length!");

tools/profiler/core/ProfileBuffer.cpp

@@ -68,7 +68,8 @@ ProfileBuffer::AddStoredMarker(ProfilerMarker *aStoredMarker)
 
 void
 ProfileBuffer::CollectCodeLocation(
-  const char* aLabel, const char* aStr, int aLineNumber,
+  const char* aLabel, const char* aStr,
+  const Maybe<uint32_t>& aLineNumber, const Maybe<uint32_t>& aColumnNumber,
   const Maybe<js::ProfilingStackFrame::Category>& aCategory)
 {
   AddEntry(ProfileBufferEntry::Label(aLabel));
@@ -90,8 +91,12 @@ ProfileBuffer::CollectCodeLocation(
     }
   }
 
-  if (aLineNumber != -1) {
-    AddEntry(ProfileBufferEntry::LineNumber(aLineNumber));
+  if (aLineNumber) {
+    AddEntry(ProfileBufferEntry::LineNumber(*aLineNumber));
+  }
+
+  if (aColumnNumber) {
+    AddEntry(ProfileBufferEntry::ColumnNumber(*aColumnNumber));
   }
 
   if (aCategory.isSome()) {
@@ -149,7 +154,7 @@ ProfileBufferCollector::CollectJitReturnAddr(void* aAddr)
 void
 ProfileBufferCollector::CollectWasmFrame(const char* aLabel)
 {
-  mBuf.CollectCodeLocation("", aLabel, -1, Nothing());
+  mBuf.CollectCodeLocation("", aLabel, Nothing(), Nothing(), Nothing());
 }
 
 void
@@ -163,7 +168,8 @@ ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame
   const char* label = aFrame.label();
   const char* dynamicString = aFrame.dynamicString();
   bool isChromeJSEntry = false;
-  int lineno = -1;
+  Maybe<uint32_t> line;
+  Maybe<uint32_t> column;
 
   if (aFrame.isJsFrame()) {
     // There are two kinds of JS frames that get pushed onto the ProfilingStack.
@@ -181,7 +187,9 @@ ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame
       if (aFrame.script()) {
         isChromeJSEntry = IsChromeJSScript(aFrame.script());
         if (aFrame.pc()) {
-          lineno = JS_PCToLineNumber(aFrame.script(), aFrame.pc());
+          unsigned col = 0;
+          line = Some(JS_PCToLineNumber(aFrame.script(), aFrame.pc(), &col));
+          column = Some(col);
         }
       }
 
@@ -190,7 +198,7 @@ ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame
     }
   } else {
     MOZ_ASSERT(aFrame.isLabelFrame());
-    lineno = aFrame.line();
+    line = Some(aFrame.line());
   }
 
   if (dynamicString) {
@@ -202,6 +210,6 @@ ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame
     }
   }
 
-  mBuf.CollectCodeLocation(label, dynamicString, lineno,
+  mBuf.CollectCodeLocation(label, dynamicString, line, column,
                            Some(aFrame.category()));
 }

tools/profiler/core/ProfileBuffer.h

@@ -44,7 +44,9 @@ public:
   uint64_t AddThreadIdEntry(int aThreadId);
 
   void CollectCodeLocation(
-    const char* aLabel, const char* aStr, int aLineNumber,
+    const char* aLabel, const char* aStr,
+    const mozilla::Maybe<uint32_t>& aLineNumber,
+    const mozilla::Maybe<uint32_t>& aColumnNumber,
     const mozilla::Maybe<js::ProfilingStackFrame::Category>& aCategory);
 
   // Maximum size of a frameKey string that we'll handle.

tools/profiler/core/ProfileBufferEntry.cpp

@@ -343,6 +343,7 @@ UniqueStacks::FrameKey::NormalFrameData::operator==(const NormalFrameData& aOthe
 {
   return mLocation == aOther.mLocation &&
          mLine == aOther.mLine &&
+         mColumn == aOther.mColumn &&
          mCategory == aOther.mCategory;
 }
 
@@ -366,6 +367,9 @@ UniqueStacks::FrameKey::Hash() const
     if (data.mLine.isSome()) {
       hash = AddToHash(hash, *data.mLine);
     }
+    if (data.mColumn.isSome()) {
+      hash = AddToHash(hash, *data.mColumn);
+    }
     if (data.mCategory.isSome()) {
       hash = AddToHash(hash, *data.mCategory);
     }
@@ -508,7 +512,8 @@ UniqueStacks::StreamNonJITFrame(const FrameKey& aFrame)
     IMPLEMENTATION = 1,
     OPTIMIZATIONS = 2,
     LINE = 3,
-    CATEGORY = 4
+    COLUMN = 4,
+    CATEGORY = 5
   };
 
   AutoArraySchemaWriter writer(mFrameTableWriter, *mUniqueStrings);
@@ -518,6 +523,9 @@ UniqueStacks::StreamNonJITFrame(const FrameKey& aFrame)
   if (data.mLine.isSome()) {
     writer.IntElement(LINE, *data.mLine);
   }
+  if (data.mColumn.isSome()) {
+    writer.IntElement(COLUMN, *data.mColumn);
+  }
   if (data.mCategory.isSome()) {
     writer.IntElement(CATEGORY, *data.mCategory);
   }
@@ -626,7 +634,8 @@ StreamJITFrame(JSContext* aContext, SpliceableJSONWriter& aWriter,
     IMPLEMENTATION = 1,
     OPTIMIZATIONS = 2,
     LINE = 3,
-    CATEGORY = 4
+    COLUMN = 4,
+    CATEGORY = 5
   };
 
   AutoArraySchemaWriter writer(aWriter, aUniqueStrings);
@@ -1019,6 +1028,12 @@ ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
           e.Next();
         }
 
+        Maybe<unsigned> column;
+        if (e.Has() && e.Get().IsColumnNumber()) {
+          column = Some(unsigned(e.Get().u.mInt));
+          e.Next();
+        }
+
         Maybe<unsigned> category;
         if (e.Has() && e.Get().IsCategory()) {
           category = Some(unsigned(e.Get().u.mInt));
@@ -1026,7 +1041,7 @@ ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
         }
 
         stack = aUniqueStacks.AppendFrame(
-          stack, UniqueStacks::FrameKey(strbuf.get(), line, category));
+          stack, UniqueStacks::FrameKey(strbuf.get(), line, column, category));
 
       } else if (e.Get().IsJitReturnAddr()) {
         numFrames++;

tools/profiler/core/ProfileBufferEntry.h

@@ -38,6 +38,7 @@ class ProfilerMarker;
   macro(DynamicStringFragment, char*) /* char[kNumChars], really */ \
   macro(JitReturnAddr,         void*) \
   macro(LineNumber,            int) \
+  macro(ColumnNumber,          int) \
   macro(NativeLeafAddr,        void*) \
   macro(Marker,                ProfilerMarker*) \
   macro(Pause,                 double) \
@@ -224,8 +225,9 @@ public:
     }
 
     FrameKey(const char* aLocation, const mozilla::Maybe<unsigned>& aLine,
+             const mozilla::Maybe<unsigned>& aColumn,
              const mozilla::Maybe<unsigned>& aCategory)
-      : mData(NormalFrameData{ nsCString(aLocation), aLine, aCategory })
+      : mData(NormalFrameData{ nsCString(aLocation), aLine, aColumn, aCategory })
     {
     }
 
@@ -244,6 +246,7 @@ public:
 
       nsCString mLocation;
       mozilla::Maybe<unsigned> mLine;
+      mozilla::Maybe<unsigned> mColumn;
       mozilla::Maybe<unsigned> mCategory;
     };
     struct JITFrameData {
@@ -396,7 +399,8 @@ private:
 //       "implementation": 1,  /* index into stringTable */
 //       "optimizations": 2,   /* arbitrary JSON */
 //       "line": 3,            /* number */
-//       "category": 4         /* number */
+//       "column": 4,          /* number */
+//       "category": 5         /* number */
 //     },
 //     "data":
 //     [

tools/profiler/core/ProfiledThreadData.cpp

@@ -87,6 +87,7 @@ ProfiledThreadData::StreamJSON(const ProfileBuffer& aBuffer, JSContext* aCx,
         schema.WriteField("implementation");
         schema.WriteField("optimizations");
         schema.WriteField("line");
+        schema.WriteField("column");
         schema.WriteField("category");
       }
 

tools/profiler/core/platform.cpp

@@ -1627,7 +1627,7 @@ StreamMetaJSCustomObject(PSLockRef aLock, SpliceableJSONWriter& aWriter,
 {
   MOZ_RELEASE_ASSERT(CorePS::Exists() && ActivePS::Exists(aLock));
 
-  aWriter.IntProperty("version", 11);
+  aWriter.IntProperty("version", 12);
 
   // The "startTime" field holds the number of milliseconds since midnight
   // January 1, 1970 GMT. This grotty code computes (Now - (Now -
@@ -1824,7 +1824,8 @@ CollectJavaThreadProfileData()
         parentFrameWasIdleFrame = false;
       }
 
-      buffer->CollectCodeLocation("", frameNameString.get(), -1, category);
+      buffer->CollectCodeLocation("", frameNameString.get(), Nothing(),
+          Nothing(), category);
     }
     sampleId++;
   }