Bug 1499507 - Use AppendPrintf to concatenate the label with the dynamic string. r=njn
☠☠ backed out by ccf46ccf3519 ☠ ☠
authorMarkus Stange <mstange@themasta.com>
Mon, 05 Nov 2018 20:55:38 +0000
changeset 444472 6256446f16c4e3b4c871504e7bbeca69871f2c3d
parent 444471 41cede6bc7d219f05c984acc95649283c81d8287
child 444473 76dd85b9aaf7d550b23585b4be898b03e4811e27
push id34996
push userrgurzau@mozilla.com
push dateTue, 06 Nov 2018 09:53:23 +0000
treeherdermozilla-central@e160f0a60e4f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1499507
milestone65.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 1499507 - Use AppendPrintf to concatenate the label with the dynamic string. r=njn This code is run during JSON serialization so performance is not a big concern. Depends on D9195 Differential Revision: https://phabricator.services.mozilla.com/D9197
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/ProfileBufferEntry.h
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -944,17 +944,17 @@ private:
     continue; \
   }
 
 void
 ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId,
                                    double aSinceTime,
                                    UniqueStacks& aUniqueStacks) const
 {
-  UniquePtr<char[]> strbuf = MakeUnique<char[]>(kMaxFrameKeyLength);
+  UniquePtr<char[]> dynStrBuf = MakeUnique<char[]>(kMaxFrameKeyLength);
 
   EntryGetter e(*this);
 
   for (;;) {
     // This block skips entries until we find the start of the next sample.
     // This is useful in three situations.
     //
     // - The circular buffer overwrites old entries, so when we start parsing
@@ -1019,49 +1019,47 @@ ProfileBuffer::StreamSamplesToJSON(Splic
         char buf[20];
         SprintfLiteral(buf, "%#llx", pc);
         stack = aUniqueStacks.AppendFrame(stack, UniqueStacks::FrameKey(buf));
         e.Next();
 
       } else if (e.Get().IsLabel()) {
         numFrames++;
 
-        // Copy the label into strbuf.
         const char* label = e.Get().u.mString;
-        strncpy(strbuf.get(), label, kMaxFrameKeyLength);
-        size_t i = strlen(label);
+
         e.Next();
 
-        bool seenFirstDynamicStringFragment = false;
+        // Copy potential dynamic string fragments into dynStrBuf, so that
+        // dynStrBuf will then contain the entire dynamic string.
+        size_t i = 0;
+        dynStrBuf[0] = '\0';
         while (e.Has()) {
           if (e.Get().IsDynamicStringFragment()) {
-            // If this is the first dynamic string fragment and we have a
-            // non-empty label, insert a ' ' after the label and before the
-            // dynamic string.
-            if (!seenFirstDynamicStringFragment) {
-              if (i > 0 && i < kMaxFrameKeyLength) {
-                strbuf[i] = ' ';
-                i++;
-              }
-              seenFirstDynamicStringFragment = true;
-            }
-
             for (size_t j = 0; j < ProfileBufferEntry::kNumChars; j++) {
               const char* chars = e.Get().u.mChars;
               if (i < kMaxFrameKeyLength) {
-                strbuf[i] = chars[j];
+                dynStrBuf[i] = chars[j];
                 i++;
               }
             }
             e.Next();
           } else {
             break;
           }
         }
-        strbuf[kMaxFrameKeyLength - 1] = '\0';
+        dynStrBuf[kMaxFrameKeyLength - 1] = '\0';
+        bool hasDynamicString = (i != 0);
+
+        nsCString frameLabel;
+        if (label[0] != '\0' && hasDynamicString) {
+          frameLabel.AppendPrintf("%s %s", label, dynStrBuf.get());
+        } else {
+          frameLabel.Append(label);
+        }
 
         Maybe<unsigned> line;
         if (e.Has() && e.Get().IsLineNumber()) {
           line = Some(unsigned(e.Get().u.mInt));
           e.Next();
         }
 
         Maybe<unsigned> column;
@@ -1072,17 +1070,18 @@ ProfileBuffer::StreamSamplesToJSON(Splic
 
         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, column, category));
+          stack, UniqueStacks::FrameKey(std::move(frameLabel), 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
@@ -230,20 +230,20 @@ class UniqueStacks
 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,
+    FrameKey(nsCString&& aLocation, const mozilla::Maybe<unsigned>& aLine,
              const mozilla::Maybe<unsigned>& aColumn,
              const mozilla::Maybe<unsigned>& aCategory)
-      : mData(NormalFrameData{ nsCString(aLocation), aLine, aColumn, aCategory })
+      : mData(NormalFrameData{ aLocation, aLine, aColumn, aCategory })
     {
     }
 
     FrameKey(void* aJITAddress, uint32_t aJITDepth, uint32_t aRangeIndex)
       : mData(JITFrameData{ aJITAddress, aJITDepth, aRangeIndex })
     {
     }