bug 785922: Emit column numbers for JS frames and functions in the gecko profiler r=sfink,mstange
authorDenis Palmeiro <dpalmeiro@mozilla.com>
Fri, 17 Aug 2018 19:45:39 +0000
changeset 487294 ef6a3b493405a675b56bdfed1f9480600b58ed74
parent 487293 1c0e36dd2c45a14cfbf1168223cb5080617c2b0e
child 487295 dc054d9df7a50d89f3db27fa8f951232fcbd5c73
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, mstange
bugs785922
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
js/src/jit/JitcodeMap.cpp
js/src/vm/GeckoProfiler.cpp
tools/profiler/core/ProfileBuffer.cpp
tools/profiler/core/ProfileBuffer.h
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/ProfileBufferEntry.h
tools/profiler/core/ProfiledThreadData.cpp
tools/profiler/core/platform.cpp
--- a/js/src/jit/JitcodeMap.cpp
+++ b/js/src/jit/JitcodeMap.cpp
@@ -319,39 +319,41 @@ JitcodeGlobalEntry::createScriptString(J
         nameLength = strlen(nameStr.get());
         hasName = true;
     }
 
     // Calculate filename length
     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;
     }
 
     // Allocate string.
     char* str = cx->pod_malloc<char>(fullLength + 1);
     if (!str)
         return nullptr;
@@ -365,21 +367,21 @@ JitcodeGlobalEntry::createScriptString(J
         str[cur++] = ' ';
         str[cur++] = '(';
     }
 
     // Fill string with filename chars.
     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.
     if (hasName)
         str[cur++] = ')';
 
     MOZ_ASSERT(cur == fullLength);
     str[cur] = 0;
--- a/js/src/vm/GeckoProfiler.cpp
+++ b/js/src/vm/GeckoProfiler.cpp
@@ -270,41 +270,48 @@ GeckoProfilerRuntime::allocProfileString
 
     // Get the script filename, if any, and its length.
     const char* filename = script->filename();
     if (filename == nullptr)
         filename = "<unknown>";
     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.
     }
 
     // Allocate the buffer.
     UniqueChars cstr(js_pod_malloc<char>(len + 1));
     if (!cstr)
         return nullptr;
 
     // Construct the descriptive string.
     DebugOnly<size_t> ret;
     if (atom) {
         UniqueChars atomStr = StringToNewUTF8CharsZ(nullptr, *atom);
         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!");
 
     return cstr;
 }
 
 void
--- a/tools/profiler/core/ProfileBuffer.cpp
+++ b/tools/profiler/core/ProfileBuffer.cpp
@@ -63,17 +63,18 @@ void
 ProfileBuffer::AddStoredMarker(ProfilerMarker *aStoredMarker)
 {
   aStoredMarker->SetPositionInBuffer(mRangeEnd);
   mStoredMarkers.insert(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));
 
   if (aStr) {
     // Store the string using one or more DynamicStringFragment entries.
     size_t strLen = strlen(aStr) + 1;   // +1 for the null terminator
     for (size_t j = 0; j < strLen; ) {
@@ -85,18 +86,22 @@ ProfileBuffer::CollectCodeLocation(
       }
       memcpy(chars, &aStr[j], len);
       j += ProfileBufferEntry::kNumChars;
 
       AddEntry(ProfileBufferEntry::DynamicStringFragment(chars));
     }
   }
 
-  if (aLineNumber != -1) {
-    AddEntry(ProfileBufferEntry::LineNumber(aLineNumber));
+  if (aLineNumber) {
+    AddEntry(ProfileBufferEntry::LineNumber(*aLineNumber));
+  }
+
+  if (aColumnNumber) {
+    AddEntry(ProfileBufferEntry::ColumnNumber(*aColumnNumber));
   }
 
   if (aCategory.isSome()) {
     AddEntry(ProfileBufferEntry::Category(int(*aCategory)));
   }
 }
 
 void
@@ -144,31 +149,32 @@ void
 ProfileBufferCollector::CollectJitReturnAddr(void* aAddr)
 {
   mBuf.AddEntry(ProfileBufferEntry::JitReturnAddr(aAddr));
 }
 
 void
 ProfileBufferCollector::CollectWasmFrame(const char* aLabel)
 {
-  mBuf.CollectCodeLocation("", aLabel, -1, Nothing());
+  mBuf.CollectCodeLocation("", aLabel, Nothing(), Nothing(), Nothing());
 }
 
 void
 ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame& aFrame)
 {
   // WARNING: this function runs within the profiler's "critical section".
 
   MOZ_ASSERT(aFrame.kind() == js::ProfilingStackFrame::Kind::LABEL ||
              aFrame.kind() == js::ProfilingStackFrame::Kind::JS_NORMAL);
 
   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.
     //
     // - label = "", dynamic string = <something>
     // - label = "js::RunScript", dynamic string = nullptr
     //
     // The line number is only interesting in the first case.
@@ -176,32 +182,34 @@ ProfileBufferCollector::CollectProfiling
     if (label[0] == '\0') {
       MOZ_ASSERT(dynamicString);
 
       // We call aFrame.script() repeatedly -- rather than storing the result in
       // a local variable in order -- to avoid rooting hazards.
       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);
         }
       }
 
     } else {
       MOZ_ASSERT(strcmp(label, "js::RunScript") == 0 && !dynamicString);
     }
   } else {
     MOZ_ASSERT(aFrame.isLabelFrame());
-    lineno = aFrame.line();
+    line = Some(aFrame.line());
   }
 
   if (dynamicString) {
     // Adjust the dynamic string as necessary.
     if (ProfilerFeature::HasPrivacy(mFeatures) && !isChromeJSEntry) {
       dynamicString = "(private)";
     } else if (strlen(dynamicString) >= ProfileBuffer::kMaxFrameKeyLength) {
       dynamicString = "(too long)";
     }
   }
 
-  mBuf.CollectCodeLocation(label, dynamicString, lineno,
+  mBuf.CollectCodeLocation(label, dynamicString, line, column,
                            Some(aFrame.category()));
 }
--- a/tools/profiler/core/ProfileBuffer.h
+++ b/tools/profiler/core/ProfileBuffer.h
@@ -39,17 +39,19 @@ public:
   // Add |aEntry| to the buffer, ignoring what kind of entry it is.
   void AddEntry(const ProfileBufferEntry& aEntry);
 
   // Add to the buffer a sample start (ThreadId) entry for aThreadId.
   // Returns the position of the entry.
   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.
   static const size_t kMaxFrameKeyLength = 512;
 
   // Add JIT frame information to aJITFrameInfo for any JitReturnAddr entries
   // that are currently in the buffer at or after aRangeStart, in samples
   // for the given thread.
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -338,16 +338,17 @@ JITFrameInfo::JITFrameInfo(const JITFram
   }
 }
 
 bool
 UniqueStacks::FrameKey::NormalFrameData::operator==(const NormalFrameData& aOther) const
 {
   return mLocation == aOther.mLocation &&
          mLine == aOther.mLine &&
+         mColumn == aOther.mColumn &&
          mCategory == aOther.mCategory;
 }
 
 bool
 UniqueStacks::FrameKey::JITFrameData::operator==(const JITFrameData& aOther) const
 {
   return mCanonicalAddress == aOther.mCanonicalAddress &&
          mDepth == aOther.mDepth &&
@@ -361,16 +362,19 @@ UniqueStacks::FrameKey::Hash() const
   if (mData.is<NormalFrameData>()) {
     const NormalFrameData& data = mData.as<NormalFrameData>();
     if (!data.mLocation.IsEmpty()) {
       hash = AddToHash(hash, HashString(data.mLocation.get()));
     }
     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);
     }
   } else {
     const JITFrameData& data = mData.as<JITFrameData>();
     hash = AddToHash(hash, data.mCanonicalAddress);
     hash = AddToHash(hash, data.mDepth);
     hash = AddToHash(hash, data.mRangeIndex);
@@ -503,26 +507,30 @@ UniqueStacks::StreamNonJITFrame(const Fr
 {
   using NormalFrameData = FrameKey::NormalFrameData;
 
   enum Schema : uint32_t {
     LOCATION = 0,
     IMPLEMENTATION = 1,
     OPTIMIZATIONS = 2,
     LINE = 3,
-    CATEGORY = 4
+    COLUMN = 4,
+    CATEGORY = 5
   };
 
   AutoArraySchemaWriter writer(mFrameTableWriter, *mUniqueStrings);
 
   const NormalFrameData& data = aFrame.mData.as<NormalFrameData>();
   writer.StringElement(LOCATION, data.mLocation.get());
   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);
   }
 }
 
 static void
 StreamJITFrameOptimizations(SpliceableJSONWriter& aWriter,
                             UniqueJSONStrings& aUniqueStrings,
@@ -621,17 +629,18 @@ StreamJITFrame(JSContext* aContext, Spli
                UniqueJSONStrings& aUniqueStrings,
                const JS::ProfiledFrameHandle& aJITFrame)
 {
   enum Schema : uint32_t {
     LOCATION = 0,
     IMPLEMENTATION = 1,
     OPTIMIZATIONS = 2,
     LINE = 3,
-    CATEGORY = 4
+    COLUMN = 4,
+    CATEGORY = 5
   };
 
   AutoArraySchemaWriter writer(aWriter, aUniqueStrings);
 
   writer.StringElement(LOCATION, aJITFrame.label());
 
   JS::ProfilingFrameIterator::FrameKind frameKind = aJITFrame.frameKind();
   MOZ_ASSERT(frameKind == JS::ProfilingFrameIterator::Frame_Ion ||
@@ -1014,24 +1023,30 @@ ProfileBuffer::StreamSamplesToJSON(Splic
         strbuf[kMaxFrameKeyLength - 1] = '\0';
 
         Maybe<unsigned> line;
         if (e.Has() && e.Get().IsLineNumber()) {
           line = Some(unsigned(e.Get().u.mInt));
           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));
           e.Next();
         }
 
         stack = aUniqueStacks.AppendFrame(
-          stack, UniqueStacks::FrameKey(strbuf.get(), line, category));
+          stack, UniqueStacks::FrameKey(strbuf.get(), line, column, category));
 
       } else if (e.Get().IsJitReturnAddr()) {
         numFrames++;
 
         // A JIT frame may expand to multiple frames due to inlining.
         void* pc = e.Get().u.mPtr;
         const Maybe<nsTArray<UniqueStacks::FrameKey>>& frameKeys =
           aUniqueStacks.LookupFramesForJITAddressFromBufferPos(pc, e.CurPos());
--- a/tools/profiler/core/ProfileBufferEntry.h
+++ b/tools/profiler/core/ProfileBufferEntry.h
@@ -33,16 +33,17 @@ class ProfilerMarker;
 #define FOR_EACH_PROFILE_BUFFER_ENTRY_KIND(macro) \
   macro(Category,              int) \
   macro(CollectionStart,       double) \
   macro(CollectionEnd,         double) \
   macro(Label,                 const char*) \
   macro(DynamicStringFragment, char*) /* char[kNumChars], really */ \
   macro(JitReturnAddr,         void*) \
   macro(LineNumber,            int) \
+  macro(ColumnNumber,          int) \
   macro(NativeLeafAddr,        void*) \
   macro(Marker,                ProfilerMarker*) \
   macro(Pause,                 double) \
   macro(ResidentMemory,        double) \
   macro(Responsiveness,        double) \
   macro(Resume,                double) \
   macro(ThreadId,              int) \
   macro(Time,                  double) \
@@ -219,18 +220,19 @@ public:
   struct FrameKey {
     explicit FrameKey(const char* aLocation)
       : mData(NormalFrameData{
                 nsCString(aLocation), mozilla::Nothing(), mozilla::Nothing() })
     {
     }
 
     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 })
     {
     }
 
     FrameKey(void* aJITAddress, uint32_t aJITDepth, uint32_t aRangeIndex)
       : mData(JITFrameData{ aJITAddress, aJITDepth, aRangeIndex })
     {
     }
 
@@ -239,16 +241,17 @@ public:
     uint32_t Hash() const;
     bool operator==(const FrameKey& aOther) const { return mData == aOther.mData; }
 
     struct NormalFrameData {
       bool operator==(const NormalFrameData& aOther) const;
 
       nsCString mLocation;
       mozilla::Maybe<unsigned> mLine;
+      mozilla::Maybe<unsigned> mColumn;
       mozilla::Maybe<unsigned> mCategory;
     };
     struct JITFrameData {
       bool operator==(const JITFrameData& aOther) const;
 
       void* mCanonicalAddress;
       uint32_t mDepth;
       uint32_t mRangeIndex;
@@ -391,17 +394,18 @@ private:
 //   "frameTable":
 //   {
 //     "schema":
 //     {
 //       "location": 0,        /* index into stringTable */
 //       "implementation": 1,  /* index into stringTable */
 //       "optimizations": 2,   /* arbitrary JSON */
 //       "line": 3,            /* number */
-//       "category": 4         /* number */
+//       "column": 4,          /* number */
+//       "category": 5         /* number */
 //     },
 //     "data":
 //     [
 //       [ 0 ],                /* { location: '(root)' } */
 //       [ 1, 2 ]              /* { location: 'foo.js', implementation: 'baseline' } */
 //     ]
 //   },
 //
--- a/tools/profiler/core/ProfiledThreadData.cpp
+++ b/tools/profiler/core/ProfiledThreadData.cpp
@@ -82,16 +82,17 @@ ProfiledThreadData::StreamJSON(const Pro
     aWriter.StartObjectProperty("frameTable");
     {
       {
         JSONSchemaWriter schema(aWriter);
         schema.WriteField("location");
         schema.WriteField("implementation");
         schema.WriteField("optimizations");
         schema.WriteField("line");
+        schema.WriteField("column");
         schema.WriteField("category");
       }
 
       aWriter.StartArrayProperty("data");
       {
         uniqueStacks.SpliceFrameTableElements(aWriter);
       }
       aWriter.EndArray();
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -1622,17 +1622,17 @@ StreamCategories(SpliceableJSONWriter& a
 }
 
 static void
 StreamMetaJSCustomObject(PSLockRef aLock, SpliceableJSONWriter& aWriter,
                          bool aIsShuttingDown)
 {
   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 -
   // ProcessStartTime)) to convert CorePS::ProcessStartTime() into that form.
   TimeDuration delta = TimeStamp::Now() - CorePS::ProcessStartTime();
   aWriter.DoubleProperty(
     "startTime", static_cast<double>(PR_Now()/1000.0 - delta.ToMilliseconds()));
 
@@ -1819,17 +1819,18 @@ CollectJavaThreadProfileData()
       if (frameNameString.EqualsLiteral("android.os.MessageQueue.nativePollOnce()")) {
         category = Some(js::ProfilingStackFrame::Category::IDLE);
         parentFrameWasIdleFrame = true;
       } else if (parentFrameWasIdleFrame) {
         category = Some(js::ProfilingStackFrame::Category::OTHER);
         parentFrameWasIdleFrame = false;
       }
 
-      buffer->CollectCodeLocation("", frameNameString.get(), -1, category);
+      buffer->CollectCodeLocation("", frameNameString.get(), Nothing(),
+          Nothing(), category);
     }
     sampleId++;
   }
   return buffer;
 }
 #endif
 
 static void