Bug 1248094 - Simplify PCLocationMap with GCHashMap; r=fitzgen
authorTerrence Cole <terrence@mozilla.com>
Tue, 16 Feb 2016 07:54:28 -0800
changeset 284364 3418b24c9e474b04ac0bd6290416fcfe55212d38
parent 284363 1a281e177105467ba648d0fa717cc82cba4f2fb8
child 284365 99dae519223a2e4f6c0df9824b5117726afbd98d
push id71929
push usertcole@mozilla.com
push dateTue, 16 Feb 2016 16:07:48 +0000
treeherdermozilla-inbound@3418b24c9e47 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1248094
milestone47.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 1248094 - Simplify PCLocationMap with GCHashMap; r=fitzgen
js/src/jscompartment.cpp
js/src/vm/SavedStacks.cpp
js/src/vm/SavedStacks.h
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -668,17 +668,17 @@ void
 JSCompartment::sweepInnerViews()
 {
     innerViews.sweep();
 }
 
 void
 JSCompartment::sweepSavedStacks()
 {
-    savedStacks_.sweep(runtimeFromAnyThread());
+    savedStacks_.sweep();
 }
 
 void
 JSCompartment::sweepGlobalObject(FreeOp* fop)
 {
     if (global_ && IsAboutToBeFinalized(&global_)) {
         if (isDebuggee())
             Debugger::detachAllDebuggersFromGlobal(fop, global_.unbarrieredGet());
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -1035,20 +1035,20 @@ SavedStacks::copyAsyncStack(JSContext* c
     MOZ_ASSERT(asyncStackObj);
     MOZ_ASSERT(js::SavedFrame::isSavedFrameAndNotProto(*asyncStackObj));
     RootedSavedFrame frame(cx, &asyncStackObj->as<js::SavedFrame>());
 
     return adoptAsyncStack(cx, frame, asyncCause, adoptedStack, maxFrameCount);
 }
 
 void
-SavedStacks::sweep(JSRuntime* rt)
+SavedStacks::sweep()
 {
     frames.sweep();
-    sweepPCLocationMap();
+    pcLocationMap.sweep();
 }
 
 void
 SavedStacks::trace(JSTracer* trc)
 {
     if (pcLocationMap.initialized()) {
         // Mark each of the source strings in our pc to location cache.
         for (PCLocationMap::Enum e(pcLocationMap); !e.empty(); e.popFront()) {
@@ -1311,34 +1311,16 @@ SavedStacks::createFrameFromLookup(JSCon
     frame->initFromLookup(lookup);
 
     if (!FreezeObject(cx, frame))
         return nullptr;
 
     return frame;
 }
 
-/*
- * Remove entries from the table whose JSScript is being collected.
- */
-void
-SavedStacks::sweepPCLocationMap()
-{
-    for (PCLocationMap::Enum e(pcLocationMap); !e.empty(); e.popFront()) {
-        PCKey key = e.front().key();
-        JSScript* script = key.script.get();
-        if (IsAboutToBeFinalizedUnbarriered(&script)) {
-            e.removeFront();
-        } else if (script != key.script.get()) {
-            key.script = script;
-            e.rekeyFront(key);
-        }
-    }
-}
-
 bool
 SavedStacks::getLocation(JSContext* cx, const FrameIter& iter,
                          MutableHandle<LocationValue> locationp)
 {
     // We should only ever be caching location values for scripts in this
     // compartment. Otherwise, we would get dead cross-compartment scripts in
     // the cache because our compartment's sweep method isn't called when their
     // compartment gets collected.
--- a/js/src/vm/SavedStacks.h
+++ b/js/src/vm/SavedStacks.h
@@ -162,17 +162,17 @@ class SavedStacks {
         creatingSavedFrame(false)
     { }
 
     bool     init();
     bool     initialized() const { return frames.initialized(); }
     bool     saveCurrentStack(JSContext* cx, MutableHandleSavedFrame frame, unsigned maxFrameCount = 0);
     bool     copyAsyncStack(JSContext* cx, HandleObject asyncStack, HandleString asyncCause,
                             MutableHandleSavedFrame adoptedStack, unsigned maxFrameCount = 0);
-    void     sweep(JSRuntime* rt);
+    void     sweep();
     void     trace(JSTracer* trc);
     uint32_t count();
     void     clear();
     void     chooseSamplingProbability(JSCompartment*);
 
     // Set the sampling random number generator's state to |state0| and
     // |state1|. One or the other must be non-zero. See the comments for
     // mozilla::non_crypto::XorShift128PlusRNG::setState for details.
@@ -215,38 +215,42 @@ class SavedStacks {
     SavedFrame* getOrCreateSavedFrame(JSContext* cx, SavedFrame::HandleLookup lookup);
     SavedFrame* createFrameFromLookup(JSContext* cx, SavedFrame::HandleLookup lookup);
 
     // Cache for memoizing PCToLineNumber lookups.
 
     struct PCKey {
         PCKey(JSScript* script, jsbytecode* pc) : script(script), pc(pc) { }
 
-        PreBarrieredScript script;
-        jsbytecode*        pc;
+        RelocatablePtrScript script;
+        jsbytecode* pc;
+
+        bool needsSweep() { return IsAboutToBeFinalized(&script); }
     };
 
   public:
     struct LocationValue {
         LocationValue() : source(nullptr), line(0), column(0) { }
         LocationValue(JSAtom* source, size_t line, uint32_t column)
-            : source(source),
-              line(line),
-              column(column)
+            : source(source), line(line), column(column)
         { }
 
-        static void trace(LocationValue* self, JSTracer* trc) { self->trace(trc); }
         void trace(JSTracer* trc) {
             if (source)
                 TraceEdge(trc, &source, "SavedStacks::LocationValue::source");
         }
 
-        PreBarrieredAtom source;
-        size_t           line;
-        uint32_t         column;
+        bool needsSweep() {
+            MOZ_ASSERT(source);
+            return !IsAboutToBeFinalized(&source);
+        }
+
+        RelocatablePtrAtom source;
+        size_t line;
+        uint32_t column;
     };
 
     template <typename Outer>
     struct LocationValueOperations {
         JSAtom* source() const { return loc().source; }
         size_t line() const { return loc().line; }
         uint32_t column() const { return loc().column; }
       private:
@@ -259,34 +263,33 @@ class SavedStacks {
         void setLine(size_t v) { loc().line = v; }
         void setColumn(uint32_t v) { loc().column = v; }
       private:
         LocationValue& loc() { return static_cast<Outer*>(this)->get(); }
     };
 
   private:
     struct PCLocationHasher : public DefaultHasher<PCKey> {
-        typedef PointerHasher<JSScript*, 3>   ScriptPtrHasher;
-        typedef PointerHasher<jsbytecode*, 3> BytecodePtrHasher;
+        using ScriptPtrHasher = DefaultHasher<JSScript*>;
+        using BytecodePtrHasher = DefaultHasher<jsbytecode*>;
 
         static HashNumber hash(const PCKey& key) {
             return mozilla::AddToHash(ScriptPtrHasher::hash(key.script),
                                       BytecodePtrHasher::hash(key.pc));
         }
 
         static bool match(const PCKey& l, const PCKey& k) {
-            return l.script == k.script && l.pc == k.pc;
+            return ScriptPtrHasher::match(l.script, k.script) &&
+                   BytecodePtrHasher::match(l.pc, k.pc);
         }
     };
 
-    typedef HashMap<PCKey, LocationValue, PCLocationHasher, SystemAllocPolicy> PCLocationMap;
-
+    using PCLocationMap = GCHashMap<PCKey, LocationValue, PCLocationHasher, SystemAllocPolicy>;
     PCLocationMap pcLocationMap;
 
-    void sweepPCLocationMap();
     bool getLocation(JSContext* cx, const FrameIter& iter, MutableHandle<LocationValue> locationp);
 };
 
 JSObject* SavedStacksMetadataCallback(JSContext* cx, JSObject* target);
 
 template <>
 class RootedBase<SavedStacks::LocationValue>
   : public SavedStacks::MutableLocationValueOperations<JS::Rooted<SavedStacks::LocationValue>>