Bug 1037152 - Use Vector<UniquePtr> instead of Vector<Scoped> for tracking heap edge names. Also use js_free to free js_strdup's allocation, not ::free. (Usually the same thing, but not for embedders.) r=jimb
authorJeff Walden <jwalden@mit.edu>
Thu, 10 Jul 2014 16:59:26 -0700
changeset 215449 9c2d9347e6ea4fc646dccfb40af2c4dcfa5eb504
parent 215448 703ff761c6af51c7c22bbd7e74449237ecad1a9c
child 215450 121c4677c64b0f7e041fec928d9a2833472c03c3
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1037152
milestone33.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 1037152 - Use Vector<UniquePtr> instead of Vector<Scoped> for tracking heap edge names. Also use js_free to free js_strdup's allocation, not ::free. (Usually the same thing, but not for embedders.) r=jimb
js/src/builtin/TestingFunctions.cpp
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -2,17 +2,17 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "builtin/TestingFunctions.h"
 
 #include "mozilla/Move.h"
-#include "mozilla/Scoped.h"
+#include "mozilla/UniquePtr.h"
 
 #include "jsapi.h"
 #include "jscntxt.h"
 #include "jsfriendapi.h"
 #include "jsgc.h"
 #include "jsobj.h"
 #ifndef JS_MORE_DETERMINISTIC
 #include "jsprf.h"
@@ -36,17 +36,17 @@
 #include "jscntxtinlines.h"
 #include "jsobjinlines.h"
 
 using namespace js;
 using namespace JS;
 
 using mozilla::ArrayLength;
 using mozilla::Move;
-using mozilla::ScopedFreePtr;
+using mozilla::UniquePtr;
 
 // If fuzzingSafe is set, remove functionality that could cause problems with
 // fuzzers. Set this via the environment variable MOZ_FUZZING_SAFE.
 static bool fuzzingSafe = false;
 
 static bool
 GetBuildConfiguration(JSContext *cx, unsigned argc, jsval *vp)
 {
@@ -1673,71 +1673,73 @@ ReportLargeAllocationFailure(JSContext *
     void *buf = cx->runtime()->onOutOfMemoryCanGC(NULL, JSRuntime::LARGE_ALLOCATION);
     js_free(buf);
     args.rval().setUndefined();
     return true;
 }
 
 namespace heaptools {
 
+typedef UniquePtr<jschar[], JS::FreePolicy> EdgeName;
+
 // An edge to a node from its predecessor in a path through the graph.
 class BackEdge {
     // The node from which this edge starts.
     JS::ubi::Node predecessor_;
 
-    // The name of this edge. We own this storage.
-    ScopedFreePtr<jschar> name_;
+    // The name of this edge.
+    EdgeName name_;
 
   public:
     BackEdge() : name_(nullptr) { }
-    // Construct an initialized back edge. Take ownership of |name|.
-    BackEdge(JS::ubi::Node predecessor, jschar *name)
-        : predecessor_(predecessor), name_(name) { }
-    BackEdge(BackEdge &&rhs) : predecessor_(rhs.predecessor_), name_(rhs.name_.forget()) { }
+    // Construct an initialized back edge, taking ownership of |name|.
+    BackEdge(JS::ubi::Node predecessor, EdgeName name)
+        : predecessor_(predecessor), name_(Move(name)) { }
+    BackEdge(BackEdge &&rhs) : predecessor_(rhs.predecessor_), name_(Move(rhs.name_)) { }
     BackEdge &operator=(BackEdge &&rhs) {
         MOZ_ASSERT(&rhs != this);
         this->~BackEdge();
         new(this) BackEdge(Move(rhs));
         return *this;
     }
 
-    jschar *forgetName() { return name_.forget(); }
+    EdgeName forgetName() { return Move(name_); }
     JS::ubi::Node predecessor() const { return predecessor_; }
 
   private:
     // No copy constructor or copying assignment.
     BackEdge(const BackEdge &) MOZ_DELETE;
     BackEdge &operator=(const BackEdge &) MOZ_DELETE;
 };
 
 // A path-finding handler class for use with JS::ubi::BreadthFirst.
 struct FindPathHandler {
     typedef BackEdge NodeData;
     typedef JS::ubi::BreadthFirst<FindPathHandler> Traversal;
 
     FindPathHandler(JS::ubi::Node start, JS::ubi::Node target,
-                    AutoValueVector &nodes, Vector<ScopedFreePtr<jschar> > &edges)
+                    AutoValueVector &nodes, Vector<EdgeName> &edges)
       : start(start), target(target), foundPath(false),
         nodes(nodes), edges(edges) { }
 
     bool
     operator()(Traversal &traversal, JS::ubi::Node origin, const JS::ubi::Edge &edge,
                BackEdge *backEdge, bool first)
     {
         // We take care of each node the first time we visit it, so there's
         // nothing to be done on subsequent visits.
         if (!first)
             return true;
 
         // Record how we reached this node. This is the last edge on a
         // shortest path to this node.
-        jschar *edgeName = js_strdup(traversal.cx, edge.name);
+        EdgeName edgeName(js_strdup(traversal.cx, edge.name));
         if (!edgeName)
             return false;
-        *backEdge = mozilla::Move(BackEdge(origin, edgeName));
+        *backEdge = mozilla::Move(BackEdge(origin, Move(edgeName)));
 
         // Have we reached our final target node?
         if (edge.referent == target) {
             // Record the path that got us here, which must be a shortest path.
             if (!recordPath(traversal))
                 return false;
             foundPath = true;
             traversal.stop();
@@ -1777,17 +1779,17 @@ struct FindPathHandler {
 
     // The nodes and edges of the path --- should we find one. The path is
     // stored in reverse order, because that's how it's easiest for us to
     // construct it:
     // - edges[i] is the name of the edge from nodes[i] to nodes[i-1].
     // - edges[0] is the name of the edge from nodes[0] to the target.
     // - The last node, nodes[n-1], is the start node.
     AutoValueVector &nodes;
-    Vector<ScopedFreePtr<jschar> > &edges;
+    Vector<EdgeName> &edges;
 };
 
 } // namespace heaptools
 
 static bool
 FindPath(JSContext *cx, unsigned argc, jsval *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
@@ -1811,17 +1813,17 @@ FindPath(JSContext *cx, unsigned argc, j
     if (!args[1].isObject() && !args[1].isString()) {
         js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE,
                                  JSDVG_SEARCH_STACK, args[0], JS::NullPtr(),
                                  "neither an object nor a string", NULL);
         return false;
     }
 
     AutoValueVector nodes(cx);
-    Vector<ScopedFreePtr<jschar> > edges(cx);
+    Vector<heaptools::EdgeName> edges(cx);
 
     {
         // We can't tolerate the GC moving things around while we're searching
         // the heap. Check that nothing we do causes a GC.
         JS::AutoCheckCannotGC autoCannotGC;
 
         JS::ubi::Node start(args[0]), target(args[1]);
 
@@ -1863,22 +1865,24 @@ FindPath(JSContext *cx, unsigned argc, j
         RootedObject obj(cx, NewBuiltinClassInstance<JSObject>(cx));
         if (!obj)
             return false;
 
         if (!JS_DefineProperty(cx, obj, "node", nodes[i],
                                JSPROP_ENUMERATE, nullptr, nullptr))
             return false;
 
-        RootedString edge(cx, NewString<CanGC>(cx, edges[i].get(), js_strlen(edges[i])));
-        if (!edge)
+        heaptools::EdgeName edgeName = Move(edges[i]);
+
+        RootedString edgeStr(cx, NewString<CanGC>(cx, edgeName.get(), js_strlen(edgeName.get())));
+        if (!edgeStr)
             return false;
-        edges[i].forget();
-        RootedValue edgeString(cx, StringValue(edge));
-        if (!JS_DefineProperty(cx, obj, "edge", edgeString,
+        edgeName.release(); // edgeStr acquired ownership
+
+        if (!JS_DefineProperty(cx, obj, "edge", edgeStr,
                                JSPROP_ENUMERATE, nullptr, nullptr))
             return false;
 
         result->setDenseElement(length - i - 1, ObjectValue(*obj));
     }
 
     args.rval().setObject(*result);
     return true;