Title: [224603] trunk
Revision
224603
Author
sbar...@apple.com
Date
2017-11-08 15:38:55 -0800 (Wed, 08 Nov 2017)

Log Message

A JSFunction's ObjectAllocationProfile should watch the poly prototype watchpoint so it can clear its object allocation profile
https://bugs.webkit.org/show_bug.cgi?id=177792

Reviewed by Yusuke Suzuki.

JSTests:

* microbenchmarks/poly-proto-clear-js-function-allocation-profile.js: Added.
(assert):
(foo.Foo.prototype.ensureX):
(foo.Foo):
(foo):
(access):

Source/_javascript_Core:

Before this patch, if a JSFunction's rare data initialized its allocation profile
before its backing Executable's poly proto watchpoint was invalidated, that
JSFunction would continue to allocate non-poly proto objects until its allocation
profile was cleared (which essentially never happens in practice). This patch
improves on this pathology. A JSFunction's rare data will now watch the poly
proto watchpoint if it's still valid and clear its allocation profile when we
detect that we should go poly proto.

* bytecode/ObjectAllocationProfile.h:
* bytecode/ObjectAllocationProfileInlines.h:
(JSC::ObjectAllocationProfile::initializeProfile):
* runtime/FunctionRareData.cpp:
(JSC::FunctionRareData::initializeObjectAllocationProfile):
(JSC::FunctionRareData::AllocationProfileClearingWatchpoint::fireInternal):
* runtime/FunctionRareData.h:
(JSC::FunctionRareData::hasAllocationProfileClearingWatchpoint const):
(JSC::FunctionRareData::createAllocationProfileClearingWatchpoint):
(JSC::FunctionRareData::AllocationProfileClearingWatchpoint::AllocationProfileClearingWatchpoint):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (224602 => 224603)


--- trunk/JSTests/ChangeLog	2017-11-08 23:22:32 UTC (rev 224602)
+++ trunk/JSTests/ChangeLog	2017-11-08 23:38:55 UTC (rev 224603)
@@ -1,3 +1,17 @@
+2017-11-08  Saam Barati  <sbar...@apple.com>
+
+        A JSFunction's ObjectAllocationProfile should watch the poly prototype watchpoint so it can clear its object allocation profile
+        https://bugs.webkit.org/show_bug.cgi?id=177792
+
+        Reviewed by Yusuke Suzuki.
+
+        * microbenchmarks/poly-proto-clear-js-function-allocation-profile.js: Added.
+        (assert):
+        (foo.Foo.prototype.ensureX):
+        (foo.Foo):
+        (foo):
+        (access):
+
 2017-11-08  Ryan Haddad  <ryanhad...@apple.com>
 
         Mark test262.yaml/test262/test/language/statements/try/tco-catch.js as passing.

Added: trunk/JSTests/microbenchmarks/poly-proto-clear-js-function-allocation-profile.js (0 => 224603)


--- trunk/JSTests/microbenchmarks/poly-proto-clear-js-function-allocation-profile.js	                        (rev 0)
+++ trunk/JSTests/microbenchmarks/poly-proto-clear-js-function-allocation-profile.js	2017-11-08 23:38:55 UTC (rev 224603)
@@ -0,0 +1,36 @@
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion")
+}
+
+function foo() {
+    class Foo {
+        ensureX() {
+            if (!this.x)
+                this.x = 22;
+            return this;
+        }
+    };
+    return Foo;
+}
+
+function access(o) {
+    return assert(o.ensureX().x === 22);
+}
+noInline(access);
+
+let ctors = [];
+
+for (let i = 0; i < 50; ++i) {
+    let ctor = foo();
+    new ctor; // warm things up
+    ctors.push(ctor);
+}
+
+let start = Date.now();
+for (let i = 0; i < 5000; ++i) {
+    for (let i = 0; i < ctors.length; ++i)
+        access(new ctors[i]);
+}
+if (false)
+    print(Date.now() - start);

Modified: trunk/Source/_javascript_Core/ChangeLog (224602 => 224603)


--- trunk/Source/_javascript_Core/ChangeLog	2017-11-08 23:22:32 UTC (rev 224602)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-11-08 23:38:55 UTC (rev 224603)
@@ -1,3 +1,29 @@
+2017-11-08  Saam Barati  <sbar...@apple.com>
+
+        A JSFunction's ObjectAllocationProfile should watch the poly prototype watchpoint so it can clear its object allocation profile
+        https://bugs.webkit.org/show_bug.cgi?id=177792
+
+        Reviewed by Yusuke Suzuki.
+
+        Before this patch, if a JSFunction's rare data initialized its allocation profile
+        before its backing Executable's poly proto watchpoint was invalidated, that
+        JSFunction would continue to allocate non-poly proto objects until its allocation
+        profile was cleared (which essentially never happens in practice). This patch
+        improves on this pathology. A JSFunction's rare data will now watch the poly
+        proto watchpoint if it's still valid and clear its allocation profile when we
+        detect that we should go poly proto.
+
+        * bytecode/ObjectAllocationProfile.h:
+        * bytecode/ObjectAllocationProfileInlines.h:
+        (JSC::ObjectAllocationProfile::initializeProfile):
+        * runtime/FunctionRareData.cpp:
+        (JSC::FunctionRareData::initializeObjectAllocationProfile):
+        (JSC::FunctionRareData::AllocationProfileClearingWatchpoint::fireInternal):
+        * runtime/FunctionRareData.h:
+        (JSC::FunctionRareData::hasAllocationProfileClearingWatchpoint const):
+        (JSC::FunctionRareData::createAllocationProfileClearingWatchpoint):
+        (JSC::FunctionRareData::AllocationProfileClearingWatchpoint::AllocationProfileClearingWatchpoint):
+
 2017-11-08  Keith Miller  <keith_mil...@apple.com>
 
         Add super sampler begin and end bytecodes.

Modified: trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfile.h (224602 => 224603)


--- trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfile.h	2017-11-08 23:22:32 UTC (rev 224602)
+++ trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfile.h	2017-11-08 23:38:55 UTC (rev 224603)
@@ -33,6 +33,8 @@
 
 namespace JSC {
 
+class FunctionRareData;
+
 class ObjectAllocationProfile {
     friend class LLIntOffsetsExtractor;
 public:
@@ -48,7 +50,7 @@
 
     bool isNull() { return !m_structure; }
 
-    void initializeProfile(VM&, JSGlobalObject*, JSCell* owner, JSObject* prototype, unsigned inferredInlineCapacity, JSFunction* constructor = nullptr);
+    void initializeProfile(VM&, JSGlobalObject*, JSCell* owner, JSObject* prototype, unsigned inferredInlineCapacity, JSFunction* constructor = nullptr, FunctionRareData* = nullptr);
 
     Structure* structure()
     {

Modified: trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfileInlines.h (224602 => 224603)


--- trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfileInlines.h	2017-11-08 23:22:32 UTC (rev 224602)
+++ trunk/Source/_javascript_Core/bytecode/ObjectAllocationProfileInlines.h	2017-11-08 23:38:55 UTC (rev 224603)
@@ -31,7 +31,7 @@
 
 namespace JSC {
 
-ALWAYS_INLINE void ObjectAllocationProfile::initializeProfile(VM& vm, JSGlobalObject* globalObject, JSCell* owner, JSObject* prototype, unsigned inferredInlineCapacity, JSFunction* constructor)
+ALWAYS_INLINE void ObjectAllocationProfile::initializeProfile(VM& vm, JSGlobalObject* globalObject, JSCell* owner, JSObject* prototype, unsigned inferredInlineCapacity, JSFunction* constructor, FunctionRareData* functionRareData)
 {
     ASSERT(!m_allocator);
     ASSERT(!m_structure);
@@ -117,8 +117,16 @@
         executable->setCachedPolyProtoStructure(vm, structure);
     } else {
         if (executable) {
-            executable->ensurePolyProtoWatchpoint();
+            ASSERT(constructor);
+            ASSERT(functionRareData);
+            InlineWatchpointSet& polyProtoWatchpointSet = executable->ensurePolyProtoWatchpoint();
             structure->ensureRareData(vm)->setSharedPolyProtoWatchpoint(executable->sharedPolyProtoWatchpoint());
+            if (polyProtoWatchpointSet.isStillValid() && !functionRareData->hasAllocationProfileClearingWatchpoint()) {
+                // If we happen to go poly proto in the future, we want to clear this particular
+                // object allocation profile so we can transition to allocating poly proto objects.
+                Watchpoint* watchpoint = functionRareData->createAllocationProfileClearingWatchpoint();
+                polyProtoWatchpointSet.add(watchpoint);
+            }
         }
 
         m_allocator = allocator;

Modified: trunk/Source/_javascript_Core/runtime/FunctionRareData.cpp (224602 => 224603)


--- trunk/Source/_javascript_Core/runtime/FunctionRareData.cpp	2017-11-08 23:22:32 UTC (rev 224602)
+++ trunk/Source/_javascript_Core/runtime/FunctionRareData.cpp	2017-11-08 23:38:55 UTC (rev 224603)
@@ -82,7 +82,7 @@
 
 void FunctionRareData::initializeObjectAllocationProfile(VM& vm, JSGlobalObject* globalObject, JSObject* prototype, size_t inlineCapacity, JSFunction* constructor)
 {
-    m_objectAllocationProfile.initializeProfile(vm, globalObject, this, prototype, inlineCapacity, constructor);
+    m_objectAllocationProfile.initializeProfile(vm, globalObject, this, prototype, inlineCapacity, constructor, this);
 }
 
 void FunctionRareData::clear(const char* reason)
@@ -92,4 +92,9 @@
     m_objectAllocationProfileWatchpoint.fireAll(*vm(), reason);
 }
 
+void FunctionRareData::AllocationProfileClearingWatchpoint::fireInternal(const FireDetail&)
+{
+    m_rareData->clear("AllocationProfileClearingWatchpoint fired.");
 }
+
+}

Modified: trunk/Source/_javascript_Core/runtime/FunctionRareData.h (224602 => 224603)


--- trunk/Source/_javascript_Core/runtime/FunctionRareData.h	2017-11-08 23:22:32 UTC (rev 224602)
+++ trunk/Source/_javascript_Core/runtime/FunctionRareData.h	2017-11-08 23:38:55 UTC (rev 224603)
@@ -97,6 +97,14 @@
     bool hasReifiedName() const { return m_hasReifiedName; }
     void setHasReifiedName() { m_hasReifiedName = true; }
 
+    bool hasAllocationProfileClearingWatchpoint() const { return !!m_allocationProfileClearingWatchpoint; }
+    Watchpoint* createAllocationProfileClearingWatchpoint()
+    {
+        RELEASE_ASSERT(!hasAllocationProfileClearingWatchpoint());
+        m_allocationProfileClearingWatchpoint = std::make_unique<AllocationProfileClearingWatchpoint>(this);
+        return m_allocationProfileClearingWatchpoint.get();
+    }
+
 protected:
     FunctionRareData(VM&);
     ~FunctionRareData();
@@ -103,6 +111,17 @@
 
 private:
 
+    class AllocationProfileClearingWatchpoint : public Watchpoint {
+    public:
+        AllocationProfileClearingWatchpoint(FunctionRareData* rareData)
+            : m_rareData(rareData)
+        { }
+    protected:
+        void fireInternal(const FireDetail&) override;
+    private:
+        FunctionRareData* m_rareData;
+    };
+
     friend class LLIntOffsetsExtractor;
 
     // Ideally, there would only be one allocation profile for subclassing but due to Reflect.construct we
@@ -122,6 +141,7 @@
     InlineWatchpointSet m_objectAllocationProfileWatchpoint;
     InternalFunctionAllocationProfile m_internalFunctionAllocationProfile;
     WriteBarrier<Structure> m_boundFunctionStructure;
+    std::unique_ptr<AllocationProfileClearingWatchpoint> m_allocationProfileClearingWatchpoint;
     bool m_hasReifiedLength { false };
     bool m_hasReifiedName { false };
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to