[JSC] LLIntPrototypeLoadAdaptiveStructureWatchpoint does not require Bag<>
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 May 2019 04:35:36 +0000 (04:35 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 May 2019 04:35:36 +0000 (04:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197645

Reviewed by Saam Barati.

Source/JavaScriptCore:

We are using HashMap<std::tuple<Structure*, const Instruction*>, Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> for LLIntPrototypeLoadAdaptiveStructureWatchpoint,
but this has several memory inefficiency.

1. Structure* and Instruction* are too large. We can just use StructureID and bytecodeOffset (unsigned).
2. While we are using Bag<>, we do not add a new LLIntPrototypeLoadAdaptiveStructureWatchpoint after constructing this Bag first. So we can
   use Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint> instead. We ensure that new entry won't be added to this Vector by making Watchpoint
   non-movable.
3. Instead of having OpGetById::Metadata&, we just hold `unsigned` bytecodeOffset, and get Metadata& from the owner CodeBlock when needed.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finalizeLLIntInlineCaches):
* bytecode/CodeBlock.h:
* bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:
(JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint):
(JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):
* bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h:
* bytecode/Watchpoint.h:
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::setupGetByIdPrototypeCache):

Source/WTF:

* WTF.xcodeproj/project.pbxproj:
* wtf/CMakeLists.txt:
* wtf/Nonmovable.h: Copied from Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h.
* wtf/Vector.h:
(WTF::minCapacity>::uncheckedConstructAndAppend):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245050 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp
Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h
Source/JavaScriptCore/bytecode/Watchpoint.h
Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
Source/WTF/ChangeLog
Source/WTF/WTF.xcodeproj/project.pbxproj
Source/WTF/wtf/CMakeLists.txt
Source/WTF/wtf/Nonmovable.h [new file with mode: 0644]
Source/WTF/wtf/Vector.h

index 778441e..048d557 100644 (file)
@@ -1,5 +1,32 @@
 2019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
 
+        [JSC] LLIntPrototypeLoadAdaptiveStructureWatchpoint does not require Bag<>
+        https://bugs.webkit.org/show_bug.cgi?id=197645
+
+        Reviewed by Saam Barati.
+
+        We are using HashMap<std::tuple<Structure*, const Instruction*>, Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> for LLIntPrototypeLoadAdaptiveStructureWatchpoint,
+        but this has several memory inefficiency.
+
+        1. Structure* and Instruction* are too large. We can just use StructureID and bytecodeOffset (unsigned).
+        2. While we are using Bag<>, we do not add a new LLIntPrototypeLoadAdaptiveStructureWatchpoint after constructing this Bag first. So we can
+           use Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint> instead. We ensure that new entry won't be added to this Vector by making Watchpoint
+           non-movable.
+        3. Instead of having OpGetById::Metadata&, we just hold `unsigned` bytecodeOffset, and get Metadata& from the owner CodeBlock when needed.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::finalizeLLIntInlineCaches):
+        * bytecode/CodeBlock.h:
+        * bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp:
+        (JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint):
+        (JSC::LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal):
+        * bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h:
+        * bytecode/Watchpoint.h:
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::setupGetByIdPrototypeCache):
+
+2019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
         JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
         https://bugs.webkit.org/show_bug.cgi?id=197479
 
index 46f3ed7..db28f67 100644 (file)
@@ -1302,7 +1302,7 @@ void CodeBlock::finalizeLLIntInlineCaches()
     // then cleared the cache without GCing in between.
     m_llintGetByIdWatchpointMap.removeIf([&] (const StructureWatchpointMap::KeyValuePairType& pair) -> bool {
         auto clear = [&] () {
-            const Instruction* instruction = std::get<1>(pair.key);
+            auto& instruction = instructions().at(std::get<1>(pair.key));
             OpcodeID opcode = instruction->opcodeID();
             if (opcode == op_get_by_id) {
                 if (Options::verboseOSR())
@@ -1312,11 +1312,11 @@ void CodeBlock::finalizeLLIntInlineCaches()
             return true;
         };
 
-        if (!vm.heap.isMarked(std::get<0>(pair.key)))
+        if (!vm.heap.isMarked(vm.heap.structureIDTable().get(std::get<0>(pair.key))))
             return clear();
 
-        for (const LLIntPrototypeLoadAdaptiveStructureWatchpoint* watchpoint : pair.value) {
-            if (!watchpoint->key().isStillLive(vm))
+        for (const LLIntPrototypeLoadAdaptiveStructureWatchpoint& watchpoint : pair.value) {
+            if (!watchpoint.key().isStillLive(vm))
                 return clear();
         }
 
index 406fca0..a4c23bf 100644 (file)
@@ -637,7 +637,7 @@ public:
         return m_llintExecuteCounter;
     }
 
-    typedef HashMap<std::tuple<Structure*, const Instruction*>, Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> StructureWatchpointMap;
+    typedef HashMap<std::tuple<StructureID, unsigned>, Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint>> StructureWatchpointMap;
     StructureWatchpointMap& llintGetByIdWatchpointMap() { return m_llintGetByIdWatchpointMap; }
 
     // Functions for controlling when tiered compilation kicks in. This
index 1bebc48..086b513 100644 (file)
 
 namespace JSC {
 
-LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock* owner, const ObjectPropertyCondition& key, OpGetById::Metadata& getByIdMetadata)
+LLIntPrototypeLoadAdaptiveStructureWatchpoint::LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock* owner, const ObjectPropertyCondition& key, unsigned bytecodeOffset)
     : m_owner(owner)
     , m_key(key)
-    , m_getByIdMetadata(getByIdMetadata)
+    , m_bytecodeOffset(bytecodeOffset)
 {
     RELEASE_ASSERT(key.watchingRequiresStructureTransitionWatchpoint());
     RELEASE_ASSERT(!key.watchingRequiresReplacementWatchpoint());
@@ -58,7 +58,8 @@ void LLIntPrototypeLoadAdaptiveStructureWatchpoint::fireInternal(VM& vm, const F
         return;
     }
 
-    clearLLIntGetByIdCache(m_getByIdMetadata);
+    auto& instruction = m_owner->instructions().at(m_bytecodeOffset);
+    clearLLIntGetByIdCache(instruction->as<OpGetById>().metadata(m_owner));
 }
 
 void LLIntPrototypeLoadAdaptiveStructureWatchpoint::clearLLIntGetByIdCache(OpGetById::Metadata& metadata)
index 9e11808..a9b1fcd 100644 (file)
@@ -33,7 +33,7 @@ namespace JSC {
 
 class LLIntPrototypeLoadAdaptiveStructureWatchpoint final : public Watchpoint {
 public:
-    LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock*, const ObjectPropertyCondition&, OpGetById::Metadata&);
+    LLIntPrototypeLoadAdaptiveStructureWatchpoint(CodeBlock*, const ObjectPropertyCondition&, unsigned bytecodeOffset);
 
     void install(VM&);
 
@@ -47,7 +47,7 @@ protected:
 private:
     CodeBlock* m_owner;
     ObjectPropertyCondition m_key;
-    OpGetById::Metadata& m_getByIdMetadata;
+    unsigned m_bytecodeOffset;
 };
 
 } // namespace JSC
index 3720cc7..77fba3e 100644 (file)
@@ -28,6 +28,7 @@
 #include <wtf/Atomics.h>
 #include <wtf/FastMalloc.h>
 #include <wtf/Noncopyable.h>
+#include <wtf/Nonmovable.h>
 #include <wtf/PrintStream.h>
 #include <wtf/ScopedLambda.h>
 #include <wtf/SentinelLinkedList.h>
@@ -91,6 +92,7 @@ class WatchpointSet;
 
 class Watchpoint : public BasicRawSentinelNode<Watchpoint> {
     WTF_MAKE_NONCOPYABLE(Watchpoint);
+    WTF_MAKE_NONMOVABLE(Watchpoint);
     WTF_MAKE_FAST_ALLOCATED;
 public:
     Watchpoint() = default;
index 7c83295..2e90c70 100644 (file)
@@ -719,19 +719,22 @@ static void setupGetByIdPrototypeCache(ExecState* exec, VM& vm, const Instructio
     if (!conditions.isValid())
         return;
 
+    unsigned bytecodeOffset = codeBlock->bytecodeOffset(pc);
     PropertyOffset offset = invalidOffset;
     CodeBlock::StructureWatchpointMap& watchpointMap = codeBlock->llintGetByIdWatchpointMap();
-    Bag<LLIntPrototypeLoadAdaptiveStructureWatchpoint> watchpoints;
+    Vector<LLIntPrototypeLoadAdaptiveStructureWatchpoint> watchpoints;
+    watchpoints.reserveInitialCapacity(conditions.size());
     for (ObjectPropertyCondition condition : conditions) {
         if (!condition.isWatchable())
             return;
         if (condition.condition().kind() == PropertyCondition::Presence)
             offset = condition.condition().offset();
-        watchpoints.add(codeBlock, condition, metadata)->install(vm);
+        watchpoints.uncheckedConstructAndAppend(codeBlock, condition, bytecodeOffset);
+        watchpoints.last().install(vm);
     }
 
     ASSERT((offset == invalidOffset) == slot.isUnset());
-    auto result = watchpointMap.add(std::make_tuple(structure, pc), WTFMove(watchpoints));
+    auto result = watchpointMap.add(std::make_tuple(structure->id(), bytecodeOffset), WTFMove(watchpoints));
     ASSERT_UNUSED(result, result.isNewEntry);
 
     ConcurrentJSLocker locker(codeBlock->m_lock);
index 6cb8134..c341077 100644 (file)
@@ -1,3 +1,16 @@
+2019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] LLIntPrototypeLoadAdaptiveStructureWatchpoint does not require Bag<>
+        https://bugs.webkit.org/show_bug.cgi?id=197645
+
+        Reviewed by Saam Barati.
+
+        * WTF.xcodeproj/project.pbxproj:
+        * wtf/CMakeLists.txt:
+        * wtf/Nonmovable.h: Copied from Source/JavaScriptCore/bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h.
+        * wtf/Vector.h:
+        (WTF::minCapacity>::uncheckedConstructAndAppend):
+
 2019-05-07  Eric Carlson  <eric.carlson@apple.com>
 
         Define media buffering policy
index 89c91b0..0ba9840 100644 (file)
                E3A32BC31FC830E2007D7E76 /* JSValueMalloc.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSValueMalloc.h; sourceTree = "<group>"; };
                E3CF76902115D6BA0091DE48 /* CompactPointerTuple.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CompactPointerTuple.h; sourceTree = "<group>"; };
                E3E158251EADA53C004A079D /* SystemFree.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SystemFree.h; sourceTree = "<group>"; };
+               E3E64F0B22813428001E55B4 /* Nonmovable.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Nonmovable.h; sourceTree = "<group>"; };
                E431CC4A21187ADB000C8A07 /* DispatchSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DispatchSPI.h; sourceTree = "<group>"; };
                E4A0AD371A96245500536DF6 /* WorkQueue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkQueue.cpp; sourceTree = "<group>"; };
                E4A0AD381A96245500536DF6 /* WorkQueue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WorkQueue.h; sourceTree = "<group>"; };
                                1A3F6BE6174ADA2100B2EEA7 /* NeverDestroyed.h */,
                                0F0D85B317234CB100338210 /* NoLock.h */,
                                A8A472D0151A825B004123FF /* Noncopyable.h */,
+                               E3E64F0B22813428001E55B4 /* Nonmovable.h */,
                                526AEC911F6B4E5C00695F5D /* NoTailCalls.h */,
                                7CEAE5AC1EA6E10F00DB6890 /* NotFound.h */,
                                A8A472D5151A825B004123FF /* NumberOfCores.cpp */,
index 5a8bb51..1990ca9 100644 (file)
@@ -139,6 +139,7 @@ set(WTF_PUBLIC_HEADERS
     NoLock.h
     NoTailCalls.h
     Noncopyable.h
+    Nonmovable.h
     NotFound.h
     NumberOfCores.h
     OSAllocator.h
diff --git a/Source/WTF/wtf/Nonmovable.h b/Source/WTF/wtf/Nonmovable.h
new file mode 100644 (file)
index 0000000..4c1034b
--- /dev/null
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All Rights Reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#define WTF_MAKE_NONMOVABLE(ClassName) \
+    private: \
+        ClassName(ClassName&&) = delete; \
+        ClassName& operator=(ClassName&&) = delete; \
+
index 05ff1a8..e4e1b62 100644 (file)
@@ -775,6 +775,7 @@ public:
 
     void uncheckedAppend(ValueType&& value) { uncheckedAppend<ValueType>(std::forward<ValueType>(value)); }
     template<typename U> void uncheckedAppend(U&&);
+    template<typename... Args> void uncheckedConstructAndAppend(Args&&...);
 
     template<typename U> void append(const U*, size_t);
     template<typename U, size_t otherCapacity> void appendVector(const Vector<U, otherCapacity>&);
@@ -1392,6 +1393,18 @@ ALWAYS_INLINE void Vector<T, inlineCapacity, OverflowHandler, minCapacity>::unch
 }
 
 template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
+template<typename... Args>
+ALWAYS_INLINE void Vector<T, inlineCapacity, OverflowHandler, minCapacity>::uncheckedConstructAndAppend(Args&&... args)
+{
+    ASSERT(size() < capacity());
+
+    asanBufferSizeWillChangeTo(m_size + 1);
+
+    new (NotNull, end()) T(std::forward<Args>(args)...);
+    ++m_size;
+}
+
+template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
 template<typename U, size_t otherCapacity>
 inline void Vector<T, inlineCapacity, OverflowHandler, minCapacity>::appendVector(const Vector<U, otherCapacity>& val)
 {