Changeset 181214 in webkit

Timestamp:

Mar 7, 2015, 2:20:54 PM (10 years ago)

Author:

commit-queue@webkit.org

Message:

Unreviewed, rolling out r181010.
​https://bugs.webkit.org/show_bug.cgi?id=142442

Broke media/video-src-invalid-poster.html (Requested by kling
on #webkit).

Reverted changeset:

"Stale entries in WeakGCMaps are keeping tons of WeakBlocks
alive unnecessarily."
​https://bugs.webkit.org/show_bug.cgi?id=142115
​http://trac.webkit.org/changeset/181010

Location:

trunk/Source

Files:

• JavaScriptCore/API/JSWeakObjectMapRefInternal.h (modified) (2 diffs)
• JavaScriptCore/API/JSWeakObjectMapRefPrivate.cpp (modified) (2 diffs)
• JavaScriptCore/API/JSWrapperMap.mm (modified) (5 diffs)
• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj (modified) (4 diffs)
• JavaScriptCore/heap/Heap.cpp (modified) (2 diffs)
• JavaScriptCore/heap/Heap.h (modified) (3 diffs)
• JavaScriptCore/heap/HeapInlines.h (modified) (1 diff)
• JavaScriptCore/runtime/JSCInlines.h (modified) (1 diff)
• JavaScriptCore/runtime/JSGlobalObject.cpp (modified) (1 diff)
• JavaScriptCore/runtime/PrototypeMap.h (modified) (1 diff)
• JavaScriptCore/runtime/Structure.cpp (modified) (2 diffs)
• JavaScriptCore/runtime/VM.cpp (modified) (2 diffs)
• JavaScriptCore/runtime/WeakGCMap.h (modified) (5 diffs)
• JavaScriptCore/runtime/WeakGCMapInlines.h (deleted)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/ForwardingHeaders/runtime/WeakGCMapInlines.h (deleted)
• WebCore/bindings/js/ScriptCachedFrameData.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/API/JSWeakObjectMapRefInternal.h

@@ -42 +42 @@
 struct OpaqueJSWeakObjectMap : public RefCounted<OpaqueJSWeakObjectMap> {
 public:
-    static Ref<OpaqueJSWeakObjectMap> create(JSC::VM& vm, void* data, JSWeakMapDestroyedCallback callback)
+    static PassRefPtr<OpaqueJSWeakObjectMap> create(void* data, JSWeakMapDestroyedCallback callback)
     {
-        return adoptRef(*new OpaqueJSWeakObjectMap(vm, data, callback));
+        return adoptRef(new OpaqueJSWeakObjectMap(data, callback));
     }
 
@@ -55 +55 @@
 
 private:
-    OpaqueJSWeakObjectMap(JSC::VM& vm, void* data, JSWeakMapDestroyedCallback callback)
-        : m_map(vm)
-        , m_data(data)
+    OpaqueJSWeakObjectMap(void* data, JSWeakMapDestroyedCallback callback)
+        : m_data(data)
         , m_callback(callback)
     {

trunk/Source/JavaScriptCore/API/JSWeakObjectMapRefPrivate.cpp

@@ -33 +33 @@
 #include "JSCInlines.h"
 #include "Weak.h"
-#include "WeakGCMapInlines.h"
 #include <wtf/HashMap.h>
 #include <wtf/text/StringHash.h>
@@ -48 +47 @@
     ExecState* exec = toJS(context);
     JSLockHolder locker(exec);
-    RefPtr<OpaqueJSWeakObjectMap> map = OpaqueJSWeakObjectMap::create(exec->vm(), privateData, callback);
+    RefPtr<OpaqueJSWeakObjectMap> map = OpaqueJSWeakObjectMap::create(privateData, callback);
     exec->lexicalGlobalObject()->registerWeakMap(map.get());
     return map.get();

trunk/Source/JavaScriptCore/API/JSWrapperMap.mm

@@ -38 +38 @@
 #import "ObjcRuntimeExtras.h"
 #import "WeakGCMap.h"
-#import "WeakGCMapInlines.h"
 #import <wtf/HashSet.h>
 #import <wtf/TCSpinLock.h>
@@ -548 +547 @@
     JSContext *m_context;
     NSMutableDictionary *m_classMap;
-    std::unique_ptr<JSC::WeakGCMap<id, JSC::JSObject>> m_cachedJSWrappers;
+    JSC::WeakGCMap<id, JSC::JSObject> m_cachedJSWrappers;
     NSMapTable *m_cachedObjCWrappers;
 }
@@ -561 +560 @@
     NSPointerFunctionsOptions valueOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
     m_cachedObjCWrappers = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
-
-    m_cachedJSWrappers = std::make_unique<JSC::WeakGCMap<id, JSC::JSObject>>(toJS([context JSGlobalContextRef])->vm());
-
+    
     m_context = context;
     m_classMap = [[NSMutableDictionary alloc] init];
@@ -594 +591 @@
 - (JSValue *)jsWrapperForObject:(id)object
 {
-    JSC::JSObject* jsWrapper = m_cachedJSWrappers->get(object);
+    JSC::JSObject* jsWrapper = m_cachedJSWrappers.get(object);
     if (jsWrapper)
         return [JSValue valueWithJSValueRef:toRef(jsWrapper) inContext:m_context];
@@ -610 +607 @@
     // (2) A long lived object may rack up many JSValues. When the contexts are released these will unprotect the associated JavaScript objects,
     //     but still, would probably nicer if we made it so that only one associated object was required, broadcasting object dealloc.
-    m_cachedJSWrappers->set(object, jsWrapper);
+    m_cachedJSWrappers.set(object, jsWrapper);
     return [JSValue valueWithJSValueRef:toRef(jsWrapper) inContext:m_context];
 }

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-03-07  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r181010.
+        https://bugs.webkit.org/show_bug.cgi?id=142442
+
+        Broke media/video-src-invalid-poster.html (Requested by kling
+        on #webkit).
+
+        Reverted changeset:
+
+        "Stale entries in WeakGCMaps are keeping tons of WeakBlocks
+        alive unnecessarily."
+        https://bugs.webkit.org/show_bug.cgi?id=142115
+        http://trac.webkit.org/changeset/181010
+
 2015-03-07  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

@@ -1436 +1436 @@
                 A7FCC26D17A0B6AA00786D1A /* FTLSwitchCase.h in Headers */ = {isa = PBXBuildFile; fileRef = A7FCC26C17A0B6AA00786D1A /* FTLSwitchCase.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 A8A4748E151A8306004123FF /* libWTF.a in Frameworks */ = {isa = PBXBuildFile; fileRef = A8A4748D151A8306004123FF /* libWTF.a */; };
-                AD86A93E1AA4D88D002FE77F /* WeakGCMapInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = AD86A93D1AA4D87C002FE77F /* WeakGCMapInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 ADDB1F6318D77DBE009B58A8 /* OpaqueRootSet.h in Headers */ = {isa = PBXBuildFile; fileRef = ADDB1F6218D77DB7009B58A8 /* OpaqueRootSet.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 ADE39FFF16DD144B0003CD4A /* PropertyTable.cpp in Sources */ = {isa = PBXBuildFile; fileRef = AD1CF06816DCAB2D00B97123 /* PropertyTable.cpp */; };
@@ -3162 +3161 @@
                 A8E894330CD0603F00367179 /* JSGlobalObject.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSGlobalObject.h; sourceTree = "<group>"; };
                 AD1CF06816DCAB2D00B97123 /* PropertyTable.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PropertyTable.cpp; sourceTree = "<group>"; };
-                AD86A93D1AA4D87C002FE77F /* WeakGCMapInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WeakGCMapInlines.h; sourceTree = "<group>"; };
                 ADDB1F6218D77DB7009B58A8 /* OpaqueRootSet.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OpaqueRootSet.h; sourceTree = "<group>"; };
                 B59F89371891AD3300D5CCDC /* UnlinkedInstructionStream.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = UnlinkedInstructionStream.h; sourceTree = "<group>"; };
@@ -4653 +4651 @@
                                 FED94F2D171E3E2300BE77A4 /* WatchdogMac.cpp */,
                                 14BFCE6810CDB1FC00364CCE /* WeakGCMap.h */,
-                                AD86A93D1AA4D87C002FE77F /* WeakGCMapInlines.h */,
                                 A7CA3ADD17DA41AE006538AF /* WeakMapConstructor.cpp */,
                                 A7CA3ADE17DA41AE006538AF /* WeakMapConstructor.h */,
@@ -5549 +5546 @@
                                 0F8023EA1613832B00A0BA45 /* ByValInfo.h in Headers */,
                                 BC18C3ED0E16F5CD00B34460 /* CallData.h in Headers */,
-                                AD86A93E1AA4D88D002FE77F /* WeakGCMapInlines.h in Headers */,
                                 1429D8DE0ED2205B00B89619 /* CallFrame.h in Headers */,
                                 A7C1EAEF17987AB600299DB2 /* CallFrameInlines.h in Headers */,

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -1067 +1067 @@
 
     reapWeakHandles();
-    pruneStaleEntriesFromWeakGCMaps();
     sweepArrayBuffers();
     snapshotMarkedSpace();
@@ -1179 +1178 @@
     GCPHASE(ReapingWeakHandles);
     m_objectSpace.reapWeakSets();
-}
-
-void Heap::pruneStaleEntriesFromWeakGCMaps()
-{
-    GCPHASE(PruningStaleEntriesFromWeakGCMaps);
-    if (m_operationInProgress != FullCollection)
-        return;
-    for (auto& pruneCallback : m_weakGCMaps.values())
-        pruneCallback();
 }
 

trunk/Source/JavaScriptCore/heap/Heap.h

@@ -229 +229 @@
 
     static bool isZombified(JSCell* cell) { return *(void**)cell == zombifiedBits; }
-
-    void registerWeakGCMap(void* weakGCMap, std::function<void()> pruningCallback);
-    void unregisterWeakGCMap(void* weakGCMap);
 
 private:
@@ -307 +304 @@
 
     void reapWeakHandles();
-    void pruneStaleEntriesFromWeakGCMaps();
     void sweepArrayBuffers();
     void snapshotMarkedSpace();
@@ -401 +397 @@
     unsigned m_delayedReleaseRecursionCount;
 #endif
-
-    HashMap<void*, std::function<void()>> m_weakGCMaps;
 };
 

trunk/Source/JavaScriptCore/heap/HeapInlines.h

@@ -320 +320 @@
     return *m_markListSet;
 }
-
-inline void Heap::registerWeakGCMap(void* weakGCMap, std::function<void()> pruningCallback)
-{
-    m_weakGCMaps.add(weakGCMap, WTF::move(pruningCallback));
-}
-
-inline void Heap::unregisterWeakGCMap(void* weakGCMap)
-{
-    m_weakGCMaps.remove(weakGCMap);
-}
     
 } // namespace JSC

trunk/Source/JavaScriptCore/runtime/JSCInlines.h

@@ -52 +52 @@
 #include "SlotVisitorInlines.h"
 #include "StructureInlines.h"
-#include "WeakGCMapInlines.h"
 
 #endif // JSCInlines_h

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -124 +124 @@
 #include "SymbolPrototype.h"
 #include "VariableWatchpointSetInlines.h"
-#include "WeakGCMapInlines.h"
 #include "WeakMapConstructor.h"
 #include "WeakMapPrototype.h"

trunk/Source/JavaScriptCore/runtime/PrototypeMap.h

@@ -34 +34 @@
 class JSObject;
 class Structure;
-class VM;
 
 // Tracks the canonical structure an object should be allocated with when inheriting from a given prototype.
 class PrototypeMap {
 public:
-    explicit PrototypeMap(VM& vm)
-        : m_prototypes(vm)
-        , m_structures(vm)
-    {
-    }
-
     JS_EXPORT_PRIVATE Structure* emptyObjectStructureForPrototype(JSObject*, unsigned inlineCapacity);
     void clearEmptyObjectStructureForPrototype(JSObject*, unsigned inlineCapacity);

trunk/Source/JavaScriptCore/runtime/Structure.cpp

@@ -37 +37 @@
 #include "StructureChain.h"
 #include "StructureRareDataInlines.h"
-#include "WeakGCMapInlines.h"
 #include <wtf/CommaPrinter.h>
 #include <wtf/ProcessID.h>
@@ -92 +91 @@
         // This handles the second transition being added
         // (or the first transition being despecified!)
-        setMap(new TransitionMap(vm));
+        setMap(new TransitionMap());
         add(vm, existingTransition);
     }

trunk/Source/JavaScriptCore/runtime/VM.cpp

@@ -84 +84 @@
 #include "TypeProfilerLog.h"
 #include "UnlinkedCodeBlock.h"
-#include "WeakGCMapInlines.h"
 #include "WeakMapData.h"
 #include <wtf/ProcessID.h>
@@ -155 +154 @@
     , propertyNames(nullptr)
     , emptyList(new MarkedArgumentBuffer)
-    , stringCache(*this)
-    , prototypeMap(*this)
     , keywords(std::make_unique<Keywords>(*this))
     , interpreter(0)

trunk/Source/JavaScriptCore/runtime/WeakGCMap.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2009, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -48 +48 @@
     typedef typename HashMapType::const_iterator const_iterator;
 
-    explicit WeakGCMap(VM&);
-    ~WeakGCMap();
+    WeakGCMap()
+        : m_gcThreshold(minGCThreshold)
+    {
+    }
 
     ValueArg* get(const KeyType& key) const
@@ -58 +60 @@
     AddResult set(const KeyType& key, ValueType value)
     {
+        gcMapIfNeeded();
         return m_map.set(key, WTF::move(value));
     }
@@ -63 +66 @@
     ALWAYS_INLINE AddResult add(const KeyType& key, ValueType value)
     {
+        gcMapIfNeeded();
         AddResult addResult = m_map.fastAdd(key, nullptr);
         if (!addResult.iterator->value) { // New value or found a zombie value.
@@ -100 +104 @@
     }
 
-    void pruneStaleEntries();
+private:
+    static const int minGCThreshold = 3;
 
-private:
+    NEVER_INLINE void gcMap()
+    {
+        Vector<KeyType, 4> zombies;
+
+        for (iterator it = m_map.begin(), end = m_map.end(); it != end; ++it) {
+            if (!it->value)
+                zombies.append(it->key);
+        }
+
+        for (size_t i = 0; i < zombies.size(); ++i)
+            m_map.remove(zombies[i]);
+    }
+
+    void gcMapIfNeeded()
+    {
+        if (m_map.size() < m_gcThreshold)
+            return;
+
+        gcMap();
+        m_gcThreshold = std::max(minGCThreshold, m_map.size() * 2 - 1);
+    }
+
     HashMapType m_map;
-    VM& m_vm;
+    int m_gcThreshold;
 };
+
+template<typename KeyArg, typename RawMappedArg, typename HashArg, typename KeyTraitsArg>
+const int WeakGCMap<KeyArg, RawMappedArg, HashArg, KeyTraitsArg>::minGCThreshold;
 
 } // namespace JSC

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-03-07  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r181010.
+        https://bugs.webkit.org/show_bug.cgi?id=142442
+
+        Broke media/video-src-invalid-poster.html (Requested by kling
+        on #webkit).
+
+        Reverted changeset:
+
+        "Stale entries in WeakGCMaps are keeping tons of WeakBlocks
+        alive unnecessarily."
+        https://bugs.webkit.org/show_bug.cgi?id=142115
+        http://trac.webkit.org/changeset/181010
+
 2015-03-07  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/bindings/js/ScriptCachedFrameData.cpp

@@ -39 +39 @@
 #include "PageConsoleClient.h"
 #include "PageGroup.h"
-#include "ScriptController.h"
 #include <heap/StrongInlines.h>
 #include <profiler/Profile.h>
 #include <runtime/JSLock.h>
-#include <runtime/WeakGCMapInlines.h>
+#include "ScriptController.h"
 
 using namespace JSC;