Bug 1434965 - Add AutoArraySchemaWriter::FreeFormElement so that FillUpTo can be private. r?njn draft
authorMarkus Stange <mstange@themasta.com>
Sat, 10 Feb 2018 17:41:35 -0500
changeset 754061 7913bd05dd5d0f10b1163e4db7cb64c0b8607d11
parent 754060 efea50db02a1accb1949266a04f50750603827dd
child 754062 bfff6a9a172b3bea5a10066d2e2f67a38de03cee
push id98744
push userbmo:mstange@themasta.com
push dateMon, 12 Feb 2018 19:50:57 +0000
reviewersnjn
bugs1434965
milestone60.0a1
Bug 1434965 - Add AutoArraySchemaWriter::FreeFormElement so that FillUpTo can be private. r?njn This may seem like an unnecessary increase in complexity, and it probably is. But it makes it easier to reason about lifetimes if your mind is accustomed to Rust, which would enforce a similar solution. MozReview-Commit-ID: kJZifbjCSU
tools/profiler/core/ProfileBufferEntry.cpp
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -155,71 +155,86 @@ public:
 
 // As mentioned in ProfileBufferEntry.h, the JSON format contains many
 // arrays whose elements are laid out according to various schemas to help
 // de-duplication. This RAII class helps write these arrays by keeping track of
 // the last non-null element written and adding the appropriate number of null
 // elements when writing new non-null elements. It also automatically opens and
 // closes an array element on the given JSON writer.
 //
+// You grant the AutoArraySchemaWriter exclusive access to the JSONWriter and
+// the UniqueJSONStrings objects for the lifetime of AutoArraySchemaWriter. Do
+// not access them independently while the AutoArraySchemaWriter is alive.
+// If you need to add complex objects, call FreeFormElement(), which will give
+// you temporary access to the writer.
+//
 // Example usage:
 //
 //     // Define the schema of elements in this type of array: [FOO, BAR, BAZ]
 //     enum Schema : uint32_t {
 //       FOO = 0,
 //       BAR = 1,
 //       BAZ = 2
 //     };
 //
 //     AutoArraySchemaWriter writer(someJsonWriter, someUniqueStrings);
 //     if (shouldWriteFoo) {
 //       writer.IntElement(FOO, getFoo());
 //     }
 //     ... etc ...
+//
+//     The elements need to be added in-order.
 class MOZ_RAII AutoArraySchemaWriter
 {
-  friend class AutoObjectWriter;
-
-  SpliceableJSONWriter& mJSONWriter;
-  UniqueJSONStrings&    mStrings;
-  uint32_t              mNextFreeIndex;
-
 public:
   AutoArraySchemaWriter(SpliceableJSONWriter& aWriter, UniqueJSONStrings& aStrings)
     : mJSONWriter(aWriter)
     , mStrings(aStrings)
     , mNextFreeIndex(0)
   {
     mJSONWriter.StartArrayElement(SpliceableJSONWriter::SingleLineStyle);
   }
 
   ~AutoArraySchemaWriter() {
     mJSONWriter.EndArray();
   }
 
-  void FillUpTo(uint32_t aIndex) {
-    MOZ_ASSERT(aIndex >= mNextFreeIndex);
-    mJSONWriter.NullElements(aIndex - mNextFreeIndex);
-    mNextFreeIndex = aIndex + 1;
-  }
-
   void IntElement(uint32_t aIndex, uint32_t aValue) {
     FillUpTo(aIndex);
     mJSONWriter.IntElement(aValue);
   }
 
   void DoubleElement(uint32_t aIndex, double aValue) {
     FillUpTo(aIndex);
     mJSONWriter.DoubleElement(aValue);
   }
 
   void StringElement(uint32_t aIndex, const char* aValue) {
     FillUpTo(aIndex);
     mStrings.WriteElement(mJSONWriter, aValue);
   }
+
+  // Write an element using a callback that takes a JSONWriter& and a
+  // UniqueJSONStrings&.
+  template<typename LambdaT>
+  void FreeFormElement(uint32_t aIndex, LambdaT aCallback) {
+    FillUpTo(aIndex);
+    aCallback(mJSONWriter, mStrings);
+  }
+
+private:
+  void FillUpTo(uint32_t aIndex) {
+    MOZ_ASSERT(aIndex >= mNextFreeIndex);
+    mJSONWriter.NullElements(aIndex - mNextFreeIndex);
+    mNextFreeIndex = aIndex + 1;
+  }
+
+  SpliceableJSONWriter& mJSONWriter;
+  UniqueJSONStrings& mStrings;
+  uint32_t mNextFreeIndex;
 };
 
 class StreamOptimizationAttemptsOp : public JS::ForEachTrackedOptimizationAttemptOp
 {
   SpliceableJSONWriter& mWriter;
   UniqueJSONStrings& mUniqueStrings;
 
 public:
@@ -415,16 +430,64 @@ UniqueStacks::StreamNonJITFrame(const Fr
   if (aFrame.mLine.isSome()) {
     writer.IntElement(LINE, *aFrame.mLine);
   }
   if (aFrame.mCategory.isSome()) {
     writer.IntElement(CATEGORY, *aFrame.mCategory);
   }
 }
 
+static void
+StreamJITFrameOptimizations(SpliceableJSONWriter& aWriter,
+                            UniqueJSONStrings& aUniqueStrings,
+                            JSContext* aContext,
+                            const JS::ProfiledFrameHandle& aJITFrame)
+{
+  aWriter.StartObjectElement();
+  {
+    aWriter.StartArrayProperty("types");
+    {
+      StreamOptimizationTypeInfoOp typeInfoOp(aWriter, aUniqueStrings);
+      aJITFrame.forEachOptimizationTypeInfo(typeInfoOp);
+    }
+    aWriter.EndArray();
+
+    JS::Rooted<JSScript*> script(aContext);
+    jsbytecode* pc;
+    aWriter.StartObjectProperty("attempts");
+    {
+      {
+        JSONSchemaWriter schema(aWriter);
+        schema.WriteField("strategy");
+        schema.WriteField("outcome");
+      }
+
+      aWriter.StartArrayProperty("data");
+      {
+        StreamOptimizationAttemptsOp attemptOp(aWriter, aUniqueStrings);
+        aJITFrame.forEachOptimizationAttempt(attemptOp, script.address(), &pc);
+      }
+      aWriter.EndArray();
+    }
+    aWriter.EndObject();
+
+    if (JSAtom* name = js::GetPropertyNameFromPC(script, pc)) {
+      char buf[512];
+      JS_PutEscapedFlatString(buf, mozilla::ArrayLength(buf), js::AtomToFlatString(name), 0);
+      aUniqueStrings.WriteProperty(aWriter, "propertyName", buf);
+    }
+
+    unsigned line, column;
+    line = JS_PCToLineNumber(script, pc, &column);
+    aWriter.IntProperty("line", line);
+    aWriter.IntProperty("column", column);
+  }
+  aWriter.EndObject();
+}
+
 void
 UniqueStacks::StreamJITFrame(const JS::ProfiledFrameHandle& aJITFrame)
 {
   enum Schema : uint32_t {
     LOCATION = 0,
     IMPLEMENTATION = 1,
     OPTIMIZATIONS = 2,
     LINE = 3,
@@ -439,57 +502,21 @@ UniqueStacks::StreamJITFrame(const JS::P
   MOZ_ASSERT(frameKind == JS::ProfilingFrameIterator::Frame_Ion ||
               frameKind == JS::ProfilingFrameIterator::Frame_Baseline);
   writer.StringElement(IMPLEMENTATION,
                         frameKind == JS::ProfilingFrameIterator::Frame_Ion
                         ? "ion"
                         : "baseline");
 
   if (aJITFrame.hasTrackedOptimizations()) {
-    writer.FillUpTo(OPTIMIZATIONS);
-    mFrameTableWriter.StartObjectElement();
-    {
-      mFrameTableWriter.StartArrayProperty("types");
-      {
-        StreamOptimizationTypeInfoOp typeInfoOp(mFrameTableWriter, mUniqueStrings);
-        aJITFrame.forEachOptimizationTypeInfo(typeInfoOp);
-      }
-      mFrameTableWriter.EndArray();
-
-      JS::Rooted<JSScript*> script(mContext);
-      jsbytecode* pc;
-      mFrameTableWriter.StartObjectProperty("attempts");
-      {
-        {
-          JSONSchemaWriter schema(mFrameTableWriter);
-          schema.WriteField("strategy");
-          schema.WriteField("outcome");
-        }
-
-        mFrameTableWriter.StartArrayProperty("data");
-        {
-          StreamOptimizationAttemptsOp attemptOp(mFrameTableWriter, mUniqueStrings);
-          aJITFrame.forEachOptimizationAttempt(attemptOp, script.address(), &pc);
-        }
-        mFrameTableWriter.EndArray();
-      }
-      mFrameTableWriter.EndObject();
-
-      if (JSAtom* name = js::GetPropertyNameFromPC(script, pc)) {
-        char buf[512];
-        JS_PutEscapedFlatString(buf, mozilla::ArrayLength(buf), js::AtomToFlatString(name), 0);
-        mUniqueStrings.WriteProperty(mFrameTableWriter, "propertyName", buf);
-      }
-
-      unsigned line, column;
-      line = JS_PCToLineNumber(script, pc, &column);
-      mFrameTableWriter.IntProperty("line", line);
-      mFrameTableWriter.IntProperty("column", column);
-    }
-    mFrameTableWriter.EndObject();
+    writer.FreeFormElement(OPTIMIZATIONS,
+      [&](SpliceableJSONWriter& aWriter, UniqueJSONStrings& aUniqueStrings) {
+        StreamJITFrameOptimizations(aWriter, aUniqueStrings, mContext,
+                                    aJITFrame);
+      });
   }
 }
 
 struct ProfileSample
 {
   uint32_t mStack;
   double mTime;
   Maybe<double> mResponsiveness;