Backed out changeset 824dd062b8e3 (bug 1288817) for startup precompilation bustage. r=backout on a CLOSED TREE
authorSebastian Hengst <archaeopteryx@coole-files.de>
Fri, 12 Aug 2016 20:18:10 +0200
changeset 309117 58f7284ec7fe447807ee7a0d34b21dd154a9321c
parent 309116 f710f7b260052f0dfc570b1f42f89cf17d891142
child 309118 d99ed7fbe3118be0204980a4119044ec30628382
push id31303
push userarchaeopteryx@coole-files.de
push dateFri, 12 Aug 2016 18:18:30 +0000
treeherderautoland@58f7284ec7fe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1288817
milestone51.0a1
backs out824dd062b8e356bfbef84e9b1f2ace5448faebda
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
Backed out changeset 824dd062b8e3 (bug 1288817) for startup precompilation bustage. r=backout on a CLOSED TREE
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCMaps.cpp
js/xpconnect/src/XPCMaps.h
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -821,16 +821,32 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp*
             // compartment being collected will be marked.
             //
             // Ideally, if NativeInterfaces from different compartments were
             // kept separate, we could sweep only the ones belonging to
             // compartments being collected. Currently, though, NativeInterfaces
             // are shared between compartments. This ought to be fixed.
             bool doSweep = !isCompartmentGC;
 
+            // We don't want to sweep the JSClasses at shutdown time.
+            // At this point there may be JSObjects using them that have
+            // been removed from the other maps.
+            if (!nsXPConnect::XPConnect()->IsShuttingDown()) {
+                for (auto i = self->mNativeScriptableSharedMap->Iter(); !i.Done(); i.Next()) {
+                    auto entry = static_cast<XPCNativeScriptableSharedMap::Entry*>(i.Get());
+                    XPCNativeScriptableShared* shared = entry->key;
+                    if (shared->IsMarked()) {
+                        shared->Unmark();
+                    } else if (doSweep) {
+                        delete shared;
+                        i.Remove();
+                    }
+                }
+            }
+
             if (doSweep) {
                 for (auto i = self->mClassInfo2NativeSetMap->Iter(); !i.Done(); i.Next()) {
                     auto entry = static_cast<ClassInfo2NativeSetMap::Entry*>(i.Get());
                     if (!entry->value->IsMarked())
                         i.Remove();
                 }
             }
 
--- a/js/xpconnect/src/XPCMaps.cpp
+++ b/js/xpconnect/src/XPCMaps.cpp
@@ -435,39 +435,38 @@ XPCNativeScriptableSharedMap::XPCNativeS
 bool
 XPCNativeScriptableSharedMap::GetNewOrUsed(uint32_t flags,
                                            char* name,
                                            XPCNativeScriptableInfo* si)
 {
     NS_PRECONDITION(name,"bad param");
     NS_PRECONDITION(si,"bad param");
 
-    RefPtr<XPCNativeScriptableShared> key =
-        new XPCNativeScriptableShared(flags, name, /* populate = */ false);
-    auto entry = static_cast<Entry*>(mTable.Add(key, fallible));
+    XPCNativeScriptableShared key(flags, name, /* populate = */ false);
+    auto entry = static_cast<Entry*>(mTable.Add(&key, fallible));
     if (!entry)
         return false;
 
-    RefPtr<XPCNativeScriptableShared> shared = entry->key;
+    XPCNativeScriptableShared* shared = entry->key;
 
     // XXX: this XPCNativeScriptableShared is heap-allocated, which means the
     // js::Class it contains is also heap-allocated. This causes problems for
     // memory reporting. See the comment above the BaseShape case in
     // StatsCellCallback() in js/src/vm/MemoryMetrics.cpp.
     //
     // When the code below is removed (bug 1265271) and there are no longer any
     // heap-allocated js::Class instances, the disabled code in
     // StatsCellCallback() should be reinstated.
     //
     if (!shared) {
-        shared = new XPCNativeScriptableShared(flags, key->TransferNameOwnership(),
-                                               /* populate = */ true);
-        entry->key = shared;
+        entry->key = shared =
+            new XPCNativeScriptableShared(flags, key.TransferNameOwnership(),
+                                          /* populate = */ true);
     }
-    si->SetScriptableShared(shared.forget());
+    si->SetScriptableShared(shared);
     return true;
 }
 
 /***************************************************************************/
 // implement XPCWrappedNativeProtoMap...
 
 // static
 XPCWrappedNativeProtoMap*
--- a/js/xpconnect/src/XPCMaps.h
+++ b/js/xpconnect/src/XPCMaps.h
@@ -492,18 +492,16 @@ private:
 
 /***************************************************************************/
 
 class XPCNativeScriptableSharedMap
 {
 public:
     struct Entry : public PLDHashEntryHdr
     {
-        // This is a weak reference that will be cleared
-        // in the XPCNativeScriptableShared destructor.
         XPCNativeScriptableShared* key;
 
         static PLDHashNumber
         Hash(const void* key);
 
         static bool
         Match(const PLDHashEntryHdr* entry, const void* key);
 
@@ -511,17 +509,17 @@ public:
     };
 
     static XPCNativeScriptableSharedMap* newMap(int length);
 
     bool GetNewOrUsed(uint32_t flags, char* name, XPCNativeScriptableInfo* si);
 
     inline uint32_t Count() { return mTable.EntryCount(); }
 
-    void Remove(XPCNativeScriptableShared* key) { mTable.Remove(key); }
+    PLDHashTable::Iterator Iter() { return mTable.Iter(); }
 
 private:
     XPCNativeScriptableSharedMap();    // no implementation
     explicit XPCNativeScriptableSharedMap(int size);
 private:
     PLDHashTable mTable;
 };
 
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -957,16 +957,18 @@ static const js::ObjectOps XPC_WN_Object
     nullptr,  // funToString
 };
 
 XPCNativeScriptableShared::XPCNativeScriptableShared(uint32_t aFlags,
                                                      char* aName,
                                                      bool aPopulate)
     : mFlags(aFlags)
 {
+    MOZ_COUNT_CTOR(XPCNativeScriptableShared);
+
     // Initialize the js::Class.
 
     memset(&mJSClass, 0, sizeof(mJSClass));
     mJSClass.name = aName;  // take ownership
 
     if (!aPopulate)
         return;
 
@@ -1052,31 +1054,16 @@ XPCNativeScriptableShared::XPCNativeScri
     mJSClass.ext = &XPC_WN_JSClassExtension;
 
     // Initialize the js::ObjectOps.
 
     if (mFlags.WantNewEnumerate())
         mJSClass.oOps = &XPC_WN_ObjectOpsWithEnumerate;
 }
 
-XPCNativeScriptableShared::~XPCNativeScriptableShared()
-{
-    // mJSClass.cOps will be null if |this| was created with
-    // populate=false. Otherwise, it was created with populate=true
-    // and there is a weak reference in a global map that must be
-    // removed.
-
-    if (mJSClass.cOps) {
-        XPCJSRuntime::Get()->GetNativeScriptableSharedMap()->Remove(this);
-        free((void*)mJSClass.cOps);
-    }
-
-    free((void*)mJSClass.name);
-}
-
 /***************************************************************************/
 /***************************************************************************/
 
 // Compatibility hack.
 //
 // XPConnect used to do all sorts of funny tricks to find the "correct"
 // |this| object for a given method (often to the detriment of proper
 // call/apply). When these tricks were removed, a fair amount of chrome
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1494,37 +1494,39 @@ public:
 // this inside the XPCNativeScriptableInfo (usually owned by instances of
 // XPCWrappedNativeProto. This had two problems... It was wasteful, and it
 // was a big problem when wrappers are reparented to different scopes (and
 // thus different protos (the DOM does this).
 
 class XPCNativeScriptableShared final
 {
 public:
-    NS_INLINE_DECL_REFCOUNTING(XPCNativeScriptableShared)
-
     const XPCNativeScriptableFlags& GetFlags() const { return mFlags; }
 
     const JSClass* GetJSClass() { return Jsvalify(&mJSClass); }
 
     XPCNativeScriptableShared(uint32_t aFlags, char* aName, bool aPopulate);
 
+    ~XPCNativeScriptableShared() {
+        free((void*)mJSClass.name);
+        free((void*)mJSClass.cOps);
+        MOZ_COUNT_DTOR(XPCNativeScriptableShared);
+    }
+
     char* TransferNameOwnership() {
         char* name = (char*)mJSClass.name;
         mJSClass.name = nullptr;
         return name;
     }
 
     void Mark()   { mFlags.Mark(); }
     void Unmark() { mFlags.Unmark(); }
     bool IsMarked() const { return mFlags.IsMarked(); }
 
 private:
-    ~XPCNativeScriptableShared();
-
     XPCNativeScriptableFlags mFlags;
 
     // This is an unusual js::Class instance: its name and cOps members are
     // heap-allocated, unlike all other instances for which they are statically
     // allocated. So we must free them in the destructor.
     js::Class mJSClass;
 };
 
@@ -1534,56 +1536,50 @@ private:
 
 class XPCNativeScriptableInfo final
 {
 public:
     static XPCNativeScriptableInfo*
     Construct(const XPCNativeScriptableCreateInfo* sci);
 
     nsIXPCScriptable*
-    GetCallback() const { return mCallback; }
+    GetCallback() const {return mCallback;}
 
     const XPCNativeScriptableFlags&
-    GetFlags() const { return mShared->GetFlags(); }
+    GetFlags() const      {return mShared->GetFlags();}
 
     const JSClass*
-    GetJSClass() { return mShared->GetJSClass(); }
+    GetJSClass()          {return mShared->GetJSClass();}
 
     void
-    SetScriptableShared(already_AddRefed<XPCNativeScriptableShared>&& shared) { mShared = shared; }
-
-    void Mark()
-    {
+    SetScriptableShared(XPCNativeScriptableShared* shared) {mShared = shared;}
+
+    void Mark() {
         if (mShared)
             mShared->Mark();
     }
 
     void TraceJS(JSTracer* trc) {}
     void AutoTrace(JSTracer* trc) {}
 
 protected:
     explicit XPCNativeScriptableInfo(nsIXPCScriptable* scriptable)
-        : mCallback(scriptable)
-    {
-        MOZ_COUNT_CTOR(XPCNativeScriptableInfo);
-    }
+        : mCallback(scriptable), mShared(nullptr)
+                               {MOZ_COUNT_CTOR(XPCNativeScriptableInfo);}
 public:
-    ~XPCNativeScriptableInfo()
-    {
-        MOZ_COUNT_DTOR(XPCNativeScriptableInfo);
-    }
+    ~XPCNativeScriptableInfo() {MOZ_COUNT_DTOR(XPCNativeScriptableInfo);}
 private:
 
     // disable copy ctor and assignment
     XPCNativeScriptableInfo(const XPCNativeScriptableInfo& r) = delete;
     XPCNativeScriptableInfo& operator= (const XPCNativeScriptableInfo& r) = delete;
 
 private:
-    nsCOMPtr<nsIXPCScriptable> mCallback;
-    RefPtr<XPCNativeScriptableShared> mShared;
+    nsCOMPtr<nsIXPCScriptable>  mCallback;
+    XPCNativeScriptableShared*  mShared;
 };
 
 /***************************************************************************/
 // XPCNativeScriptableCreateInfo is used in creating new wrapper and protos.
 // it abstracts out the scriptable interface pointer and the flags. After
 // creation these are factored differently using XPCNativeScriptableInfo.
 
 class MOZ_STACK_CLASS XPCNativeScriptableCreateInfo final