Attachment 361125 Details for Bug 194244 – Patch

[patch] Patch

bug-194244-20190204163501.patch (text/plain), 11.59 KB, created by Yusuke Suzuki on 2019-02-04 16:35:02 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-04 16:35:02 PST

Size: 11.59 KB

patch

obsolete

>Subversion Revision: 240944
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index abd09751c826c04d7670707787354ba961c4be1f..21b5a4cafb626d2f93dae52805cff583e52fca88 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,47 @@
>+2019-02-04  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] ExecutableToCodeBlockEdge should be smaller
>+        https://bugs.webkit.org/show_bug.cgi?id=194244
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        ExecutableToCodeBlockEdge is allocated so many times. However its memory layout is not efficient.
>+        sizeof(ExecutableToCodeBlockEdge) is 24bytes, but it discards 7bytes due to one bool m_isActive flag.
>+        Because our size classes are rounded by 16bytes, ExecutableToCodeBlockEdge takes 32bytes. So, half of
>+        it is wasted. We should fit it into 16bytes so that we can efficiently allocate it.
>+
>+        In this patch, we leverages TypeInfoMayBePrototype bit in JSTypeInfo. It is a bit special TypeInfo bit
>+        since this is per-cell bit. We rename this to TypeInfoPerCellBit, and use it as a `m_isActive` mark in
>+        ExecutableToCodeBlockEdge. In JSObject subclasses, we use it as MayBePrototype flag.
>+
>+        Since this flag is not changed in CAS style, we must not change this in concurrent threads. This is OK
>+        for ExecutableToCodeBlockEdge's m_isActive flag since this is touched on the main thread (ScriptExecutable::installCode
>+        does not touch it if it is called in non-main threads).
>+
>+        * bytecode/ExecutableToCodeBlockEdge.cpp:
>+        (JSC::ExecutableToCodeBlockEdge::finishCreation):
>+        (JSC::ExecutableToCodeBlockEdge::visitChildren):
>+        (JSC::ExecutableToCodeBlockEdge::activate):
>+        (JSC::ExecutableToCodeBlockEdge::deactivate):
>+        (JSC::ExecutableToCodeBlockEdge::isActive const):
>+        * bytecode/ExecutableToCodeBlockEdge.h:
>+        * runtime/JSCell.h:
>+        * runtime/JSCellInlines.h:
>+        (JSC::JSCell::perCellBit const):
>+        (JSC::JSCell::setPerCellBit):
>+        (JSC::JSCell::mayBePrototype const): Deleted.
>+        (JSC::JSCell::didBecomePrototype): Deleted.
>+        * runtime/JSObject.cpp:
>+        (JSC::JSObject::setPrototypeDirect):
>+        * runtime/JSObject.h:
>+        * runtime/JSObjectInlines.h:
>+        (JSC::JSObject::mayBePrototype const):
>+        (JSC::JSObject::didBecomePrototype):
>+        * runtime/JSTypeInfo.h:
>+        (JSC::TypeInfo::perCellBit):
>+        (JSC::TypeInfo::mergeInlineTypeFlags):
>+        (JSC::TypeInfo::mayBePrototype): Deleted.
>+
> 2019-02-04  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Shrink size of FunctionExecutable
>diff --git a/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.cpp b/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.cpp
>index 81bc9a98532b73739149b771b265458323d2ba80..14d2d36b47a317ac5615ae434a6219fcbabbffbb 100644
>--- a/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.cpp
>+++ b/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.cpp
>@@ -44,6 +44,12 @@ ExecutableToCodeBlockEdge* ExecutableToCodeBlockEdge::create(VM& vm, CodeBlock*
>     return result;
> }
> 
>+void ExecutableToCodeBlockEdge::finishCreation(VM& vm)
>+{
>+    Base::finishCreation(vm);
>+    ASSERT(!isActive());
>+}
>+
> void ExecutableToCodeBlockEdge::visitChildren(JSCell* cell, SlotVisitor& visitor)
> {
>     VM& vm = visitor.vm();
>@@ -59,7 +65,7 @@ void ExecutableToCodeBlockEdge::visitChildren(JSCell* cell, SlotVisitor& visitor
>     if (!codeBlock)
>         return;
>     
>-    if (!edge->m_isActive) {
>+    if (!edge->isActive()) {
>         visitor.appendUnbarriered(codeBlock);
>         return;
>     }
>@@ -131,14 +137,19 @@ void ExecutableToCodeBlockEdge::finalizeUnconditionally(VM& vm)
>     vm.executableToCodeBlockEdgesWithConstraints.remove(this);
> }
> 
>-void ExecutableToCodeBlockEdge::activate()
>+inline void ExecutableToCodeBlockEdge::activate()
>+{
>+    setPerCellBit(true);
>+}
>+
>+inline void ExecutableToCodeBlockEdge::deactivate()
> {
>-    m_isActive = true;
>+    setPerCellBit(false);
> }
> 
>-void ExecutableToCodeBlockEdge::deactivate()
>+inline bool ExecutableToCodeBlockEdge::isActive() const
> {
>-    m_isActive = false;
>+    return perCellBit();
> }
> 
> CodeBlock* ExecutableToCodeBlockEdge::deactivateAndUnwrap(ExecutableToCodeBlockEdge* edge)
>diff --git a/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.h b/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.h
>index 19e353048371a7dfff1ceeb8df8428e1bb71ffdc..91b6c8a1ca0d6d36b9aec227a8f418c894426b38 100644
>--- a/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.h
>+++ b/Source/JavaScriptCore/bytecode/ExecutableToCodeBlockEdge.h
>@@ -58,9 +58,6 @@ class ExecutableToCodeBlockEdge final : public JSCell {
>     static void visitOutputConstraints(JSCell*, SlotVisitor&);
>     void finalizeUnconditionally(VM&);
>     
>-    void activate();
>-    void deactivate();
>-    
>     static CodeBlock* unwrap(ExecutableToCodeBlockEdge* edge)
>     {
>         if (!edge)
>@@ -78,11 +75,16 @@ class ExecutableToCodeBlockEdge final : public JSCell {
>     friend class LLIntOffsetsExtractor;
> 
>     ExecutableToCodeBlockEdge(VM&, CodeBlock*);
>+
>+    void finishCreation(VM&);
>+
>+    void activate();
>+    void deactivate();
>+    bool isActive() const;
>     
>     void runConstraint(const ConcurrentJSLocker&, VM&, SlotVisitor&);
>     
>     WriteBarrier<CodeBlock> m_codeBlock;
>-    bool m_isActive { false };
> };
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/runtime/JSCell.h b/Source/JavaScriptCore/runtime/JSCell.h
>index 9e8ba60d9f621e49881fe22026cafdaa8ea7eda8..8cdb8732d9383e20b57d5d80fc3aea2462f468ed 100644
>--- a/Source/JavaScriptCore/runtime/JSCell.h
>+++ b/Source/JavaScriptCore/runtime/JSCell.h
>@@ -139,9 +139,6 @@ class JSCell : public HeapCell {
> 
>     TypeInfo::InlineTypeFlags inlineTypeFlags() const { return m_flags; }
>     
>-    bool mayBePrototype() const;
>-    void didBecomePrototype();
>-
>     const char* className(VM&) const;
> 
>     // Extracting the value.
>@@ -239,6 +236,9 @@ class JSCell : public HeapCell {
>     }
>     
>     static const TypedArrayType TypedArrayStorageType = NotTypedArray;
>+
>+    void setPerCellBit(bool);
>+    bool perCellBit() const;
> protected:
> 
>     void finishCreation(VM&);
>diff --git a/Source/JavaScriptCore/runtime/JSCellInlines.h b/Source/JavaScriptCore/runtime/JSCellInlines.h
>index f4639eb0792f9edcf3fcc4a9b8a7d95a5b9c2d64..f4abba2628b4263589337e53454eeb93bc624f07 100644
>--- a/Source/JavaScriptCore/runtime/JSCellInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSCellInlines.h
>@@ -382,16 +382,20 @@ inline bool JSCellLock::isLocked() const
>     return IndexingTypeLockAlgorithm::isLocked(*lock);
> }
> 
>-inline bool JSCell::mayBePrototype() const
>+inline bool JSCell::perCellBit() const
> {
>-    return TypeInfo::mayBePrototype(inlineTypeFlags());
>+    return TypeInfo::perCellBit(inlineTypeFlags());
> }
> 
>-inline void JSCell::didBecomePrototype()
>+inline void JSCell::setPerCellBit(bool value)
> {
>-    if (mayBePrototype())
>+    if (value == perCellBit())
>         return;
>-    m_flags |= static_cast<TypeInfo::InlineTypeFlags>(TypeInfoMayBePrototype);
>+
>+    if (value)
>+        m_flags |= static_cast<TypeInfo::InlineTypeFlags>(TypeInfoPerCellBit);
>+    else
>+        m_flags &= ~static_cast<TypeInfo::InlineTypeFlags>(TypeInfoPerCellBit);
> }
> 
> inline JSObject* JSCell::toObject(ExecState* exec, JSGlobalObject* globalObject) const
>diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
>index 036ec0d8012cb07159ba806766b18d0a5f8ebd5a..1b5c9173a0cc3760693d09026cf4b4d43f575f0e 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.cpp
>+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
>@@ -1742,7 +1742,7 @@ void JSObject::setPrototypeDirect(VM& vm, JSValue prototype)
> {
>     ASSERT(prototype);
>     if (prototype.isObject())
>-        prototype.asCell()->didBecomePrototype();
>+        asObject(prototype)->didBecomePrototype();
>     
>     if (structure(vm)->hasMonoProto()) {
>         DeferredStructureTransitionWatchpointFire deferred(vm, structure(vm));
>diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h
>index e0caaf3002d7a07e14606a3f5efb49bdcbecd41c..e8194c8c7372d3acbb828a73d49b88ddf4a8661a 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.h
>+++ b/Source/JavaScriptCore/runtime/JSObject.h
>@@ -879,6 +879,9 @@ class JSObject : public JSCell {
>     bool canPerformFastPutInline(VM&, PropertyName);
>     bool canPerformFastPutInlineExcludingProto(VM&);
> 
>+    bool mayBePrototype() const;
>+    void didBecomePrototype();
>+
>     DECLARE_EXPORT_INFO;
> 
> protected:
>diff --git a/Source/JavaScriptCore/runtime/JSObjectInlines.h b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>index ca34baa4d2405083799a6e6030f64441f7123aaf..83dc23dc8cbb5b981e8afc9147899316b7ccc31c 100644
>--- a/Source/JavaScriptCore/runtime/JSObjectInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>@@ -384,4 +384,14 @@ ALWAYS_INLINE bool JSObject::putDirectInternal(VM& vm, PropertyName propertyName
>     return true;
> }
> 
>+inline bool JSObject::mayBePrototype() const
>+{
>+    return perCellBit();
>+}
>+
>+inline void JSObject::didBecomePrototype()
>+{
>+    setPerCellBit(true);
>+}
>+
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/runtime/JSTypeInfo.h b/Source/JavaScriptCore/runtime/JSTypeInfo.h
>index fa44ab3c63a40fa4bbed2803c78a2515187aebe0..d67b441db19d97e7ec6dbf191b36b416b902601b 100644
>--- a/Source/JavaScriptCore/runtime/JSTypeInfo.h
>+++ b/Source/JavaScriptCore/runtime/JSTypeInfo.h
>@@ -43,7 +43,7 @@ static const unsigned OverridesGetCallData = 1 << 2; // Need this flag if you im
> static const unsigned OverridesGetOwnPropertySlot = 1 << 3;
> static const unsigned OverridesToThis = 1 << 4; // If this is false then this returns something other than 'this'. Non-object cells that are visible to JS have this set as do some exotic objects.
> static const unsigned HasStaticPropertyTable = 1 << 5;
>-static const unsigned TypeInfoMayBePrototype = 1 << 7; // Unlike other inline flags, this will only be set on the cell itself and will not be set on the Structure.
>+static const unsigned TypeInfoPerCellBit = 1 << 7; // Unlike other inline flags, this will only be set on the cell itself and will not be set on the Structure.
> 
> // Out of line flags.
> 
>@@ -89,7 +89,7 @@ class TypeInfo {
>     bool overridesGetOwnPropertySlot() const { return overridesGetOwnPropertySlot(inlineTypeFlags()); }
>     static bool overridesGetOwnPropertySlot(InlineTypeFlags flags) { return flags & OverridesGetOwnPropertySlot; }
>     static bool hasStaticPropertyTable(InlineTypeFlags flags) { return flags & HasStaticPropertyTable; }
>-    static bool mayBePrototype(InlineTypeFlags flags) { return flags & TypeInfoMayBePrototype; }
>+    static bool perCellBit(InlineTypeFlags flags) { return flags & TypeInfoPerCellBit; }
>     bool overridesToThis() const { return isSetOnFlags1(OverridesToThis); }
>     bool structureIsImmortal() const { return isSetOnFlags2(StructureIsImmortal); }
>     bool overridesGetPropertyNames() const { return isSetOnFlags2(OverridesGetPropertyNames); }
>@@ -117,10 +117,10 @@ class TypeInfo {
>         return OBJECT_OFFSETOF(TypeInfo, m_type);
>     }
> 
>-    // Since the Structure doesn't track TypeInfoMayBePrototype, we need to make sure we copy it.
>+    // Since the Structure doesn't track TypeInfoPerCellBit, we need to make sure we copy it.
>     static InlineTypeFlags mergeInlineTypeFlags(InlineTypeFlags structureFlags, InlineTypeFlags oldCellFlags)
>     {
>-        return structureFlags | (oldCellFlags & static_cast<InlineTypeFlags>(TypeInfoMayBePrototype));
>+        return structureFlags | (oldCellFlags & static_cast<InlineTypeFlags>(TypeInfoPerCellBit));
>     }
> 
>     InlineTypeFlags inlineTypeFlags() const { return m_flags; }

Flags:

msaboff: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194244: 361125