Attachment 346580 Details for Bug 188328 – Patch

[patch] Patch

bug-188328-20180804123910.patch (text/plain), 17.57 KB, created by Yusuke Suzuki on 2018-08-03 20:39:11 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-08-03 20:39:11 PDT

Size: 17.57 KB

patch

obsolete

>Subversion Revision: 234558
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4ce72a21d3fb58b8b43606eb20ee064f7650a472..b57c22db56cad463ef3d8ae61b1eaa42dd6285b8 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,48 @@
>+2018-08-03  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Shrink size of PropertyCondition by packing UniquedStringImpl* and Kind
>+        https://bugs.webkit.org/show_bug.cgi?id=188328
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Shrinking the size of PropertyCondition affects so much on memory consumption.
>+        For example, cnn.com can show 7000 persistent StructureStubClearingWatchpoint
>+        and 6000 LLIntPrototypeLoadAdaptiveStructureWatchpoint which have PropertyCondition
>+        in their member.
>+
>+        This patch shrinks the size of PropertyCondition by packing UniquedStringImpl* and
>+        PropertyCondition::Kind into uint64_t data in 64bit architecture. Since our address
>+        are within 48bit, we can put PropertyCondition::Kind in this unused bits.
>+        To make it easy, we add WTF::PointerWithType<PointerType, Type>, which automatically
>+        folds a pointer and 1byte type into 64bit data.
>+
>+        This change shrinks PropertyCondition from 24bytes to 16bytes.
>+
>+        * bytecode/PropertyCondition.cpp:
>+        (JSC::PropertyCondition::dumpInContext const):
>+        (JSC::PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint const):
>+        (JSC::PropertyCondition::validityRequiresImpurePropertyWatchpoint const):
>+        (JSC::PropertyCondition::isStillValid const):
>+        (JSC::PropertyCondition::isWatchableWhenValid const):
>+        * bytecode/PropertyCondition.h:
>+        (JSC::PropertyCondition::PropertyCondition):
>+        (JSC::PropertyCondition::presenceWithoutBarrier):
>+        (JSC::PropertyCondition::absenceWithoutBarrier):
>+        (JSC::PropertyCondition::absenceOfSetEffectWithoutBarrier):
>+        (JSC::PropertyCondition::equivalenceWithoutBarrier):
>+        (JSC::PropertyCondition::hasPrototypeWithoutBarrier):
>+        (JSC::PropertyCondition::operator bool const):
>+        (JSC::PropertyCondition::kind const):
>+        (JSC::PropertyCondition::uid const):
>+        (JSC::PropertyCondition::hasOffset const):
>+        (JSC::PropertyCondition::hasAttributes const):
>+        (JSC::PropertyCondition::hasPrototype const):
>+        (JSC::PropertyCondition::hasRequiredValue const):
>+        (JSC::PropertyCondition::hash const):
>+        (JSC::PropertyCondition::operator== const):
>+        (JSC::PropertyCondition::isHashTableDeletedValue const):
>+        (JSC::PropertyCondition::watchingRequiresReplacementWatchpoint const):
>+
> 2018-08-02  Saam Barati  <sbarati@apple.com>
> 
>         Reading instructionPointer from PlatformRegisters may fail when using pointer profiling
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index bdfe9bed7a84f86c0d39a34ea27eb536f3f16f56..f3ae0a290cc6ee22f48691e3606d3f4bb3a62dc9 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-08-03  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Shrink size of PropertyCondition by packing UniquedStringImpl* and Kind
>+        https://bugs.webkit.org/show_bug.cgi?id=188328
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/CMakeLists.txt:
>+        * wtf/PointerWithType.h: Added.
>+        (WTF::PointerWithType::PointerWithType):
>+        (WTF::PointerWithType::pointer const):
>+        (WTF::PointerWithType::setPointer):
>+        (WTF::PointerWithType::type const):
>+        (WTF::PointerWithType::setType):
>+
> 2018-08-02  Saam Barati  <sbarati@apple.com>
> 
>         Reading instructionPointer from PlatformRegisters may fail when using pointer tagging
>diff --git a/Source/JavaScriptCore/bytecode/PropertyCondition.cpp b/Source/JavaScriptCore/bytecode/PropertyCondition.cpp
>index 43634be09afbccf72226a79310574c7f3c2be247..3deb44c75ffdff0b96b4d61b84192b1bd2147708 100644
>--- a/Source/JavaScriptCore/bytecode/PropertyCondition.cpp
>+++ b/Source/JavaScriptCore/bytecode/PropertyCondition.cpp
>@@ -43,19 +43,19 @@ void PropertyCondition::dumpInContext(PrintStream& out, DumpContext* context) co
>         return;
>     }
>     
>-    switch (m_kind) {
>+    switch (m_header.type()) {
>     case Presence:
>-        out.print(m_kind, " of ", m_uid, " at ", offset(), " with attributes ", attributes());
>+        out.print(m_header.type(), " of ", m_header.pointer(), " at ", offset(), " with attributes ", attributes());
>         return;
>     case Absence:
>     case AbsenceOfSetEffect:
>-        out.print(m_kind, " of ", m_uid, " with prototype ", inContext(JSValue(prototype()), context));
>+        out.print(m_header.type(), " of ", m_header.pointer(), " with prototype ", inContext(JSValue(prototype()), context));
>         return;
>     case Equivalence:
>-        out.print(m_kind, " of ", m_uid, " with ", inContext(requiredValue(), context));
>+        out.print(m_header.type(), " of ", m_header.pointer(), " with ", inContext(requiredValue(), context));
>         return;
>     case HasPrototype:
>-        out.print(m_kind, " with prototype ", inContext(JSValue(prototype()), context));
>+        out.print(m_header.type(), " with prototype ", inContext(JSValue(prototype()), context));
>         return;
>     }
>     RELEASE_ASSERT_NOT_REACHED();
>@@ -81,7 +81,7 @@ bool PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint(
>         return false;
>     }
> 
>-    switch (m_kind) {
>+    switch (m_header.type()) {
>     case Presence:
>     case Absence:
>     case AbsenceOfSetEffect:
>@@ -102,7 +102,7 @@ bool PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint(
>         break;
>     }
>     
>-    switch (m_kind) {
>+    switch (m_header.type()) {
>     case Presence: {
>         unsigned currentAttributes;
>         PropertyOffset currentOffset = structure->getConcurrently(uid(), currentAttributes);
>@@ -259,7 +259,7 @@ bool PropertyCondition::validityRequiresImpurePropertyWatchpoint(Structure* stru
>     if (!*this)
>         return false;
>     
>-    switch (m_kind) {
>+    switch (m_header.type()) {
>     case Presence:
>     case Absence:
>     case Equivalence:
>@@ -281,7 +281,7 @@ bool PropertyCondition::isStillValid(Structure* structure, JSObject* base) const
>     // Currently we assume that an impure property can cause a property to appear, and can also
>     // "shadow" an existing JS property on the same object. Hence it affects both presence and
>     // absence. It doesn't affect AbsenceOfSetEffect because impure properties aren't ever setters.
>-    switch (m_kind) {
>+    switch (m_header.type()) {
>     case Absence:
>         if (structure->typeInfo().getOwnPropertySlotIsImpure() || structure->typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence())
>             return false;
>@@ -304,7 +304,7 @@ bool PropertyCondition::isWatchableWhenValid(
>     if (structure->transitionWatchpointSetHasBeenInvalidated())
>         return false;
>     
>-    switch (m_kind) {
>+    switch (m_header.type()) {
>     case Equivalence: {
>         PropertyOffset offset = structure->getConcurrently(uid());
>         
>diff --git a/Source/JavaScriptCore/bytecode/PropertyCondition.h b/Source/JavaScriptCore/bytecode/PropertyCondition.h
>index e2f6b1f22647e34581f5ae7e4c4c80cfc83b20a7..657b67d7462ca4a02b8ae891680bf21278b8b081 100644
>--- a/Source/JavaScriptCore/bytecode/PropertyCondition.h
>+++ b/Source/JavaScriptCore/bytecode/PropertyCondition.h
>@@ -27,6 +27,7 @@
> 
> #include "JSObject.h"
> #include <wtf/HashMap.h>
>+#include <wtf/PointerWithType.h>
> 
> namespace JSC {
> 
>@@ -34,24 +35,24 @@ class TrackedReferences;
> 
> class PropertyCondition {
> public:
>-    enum Kind {
>+    enum Kind : uint8_t {
>         Presence,
>         Absence,
>         AbsenceOfSetEffect,
>         Equivalence, // An adaptive watchpoint on this will be a pair of watchpoints, and when the structure transitions, we will set the replacement watchpoint on the new structure.
>         HasPrototype
>     };
>+
>+    using Header = PointerWithType<UniquedStringImpl*, Kind>;
>     
>     PropertyCondition()
>-        : m_uid(nullptr)
>-        , m_kind(Presence)
>+        : m_header(nullptr, Presence)
>     {
>         memset(&u, 0, sizeof(u));
>     }
>     
>     PropertyCondition(WTF::HashTableDeletedValueType)
>-        : m_uid(nullptr)
>-        , m_kind(Absence)
>+        : m_header(nullptr, Absence)
>     {
>         memset(&u, 0, sizeof(u));
>     }
>@@ -59,8 +60,7 @@ class PropertyCondition {
>     static PropertyCondition presenceWithoutBarrier(UniquedStringImpl* uid, PropertyOffset offset, unsigned attributes)
>     {
>         PropertyCondition result;
>-        result.m_uid = uid;
>-        result.m_kind = Presence;
>+        result.m_header = Header(uid, Presence);
>         result.u.presence.offset = offset;
>         result.u.presence.attributes = attributes;
>         return result;
>@@ -76,8 +76,7 @@ class PropertyCondition {
>     static PropertyCondition absenceWithoutBarrier(UniquedStringImpl* uid, JSObject* prototype)
>     {
>         PropertyCondition result;
>-        result.m_uid = uid;
>-        result.m_kind = Absence;
>+        result.m_header = Header(uid, Absence);
>         result.u.prototype.prototype = prototype;
>         return result;
>     }
>@@ -94,8 +93,7 @@ class PropertyCondition {
>         UniquedStringImpl* uid, JSObject* prototype)
>     {
>         PropertyCondition result;
>-        result.m_uid = uid;
>-        result.m_kind = AbsenceOfSetEffect;
>+        result.m_header = Header(uid, AbsenceOfSetEffect);
>         result.u.prototype.prototype = prototype;
>         return result;
>     }
>@@ -112,8 +110,7 @@ class PropertyCondition {
>         UniquedStringImpl* uid, JSValue value)
>     {
>         PropertyCondition result;
>-        result.m_uid = uid;
>-        result.m_kind = Equivalence;
>+        result.m_header = Header(uid, Equivalence);
>         result.u.equivalence.value = JSValue::encode(value);
>         return result;
>     }
>@@ -129,7 +126,7 @@ class PropertyCondition {
>     static PropertyCondition hasPrototypeWithoutBarrier(JSObject* prototype)
>     {
>         PropertyCondition result;
>-        result.m_kind = HasPrototype;
>+        result.m_header = Header(nullptr, HasPrototype);
>         result.u.prototype.prototype = prototype;
>         return result;
>     }
>@@ -141,18 +138,18 @@ class PropertyCondition {
>         return hasPrototypeWithoutBarrier(prototype);
>     }
>     
>-    explicit operator bool() const { return m_uid || m_kind != Presence; }
>+    explicit operator bool() const { return m_header.pointer() || m_header.type() != Presence; }
>     
>-    Kind kind() const { return m_kind; }
>-    UniquedStringImpl* uid() const { return m_uid; }
>+    Kind kind() const { return m_header.type(); }
>+    UniquedStringImpl* uid() const { return m_header.pointer(); }
>     
>-    bool hasOffset() const { return !!*this && m_kind == Presence; };
>+    bool hasOffset() const { return !!*this && m_header.type() == Presence; };
>     PropertyOffset offset() const
>     {
>         ASSERT(hasOffset());
>         return u.presence.offset;
>     }
>-    bool hasAttributes() const { return !!*this && m_kind == Presence; };
>+    bool hasAttributes() const { return !!*this && m_header.type() == Presence; };
>     unsigned attributes() const
>     {
>         ASSERT(hasAttributes());
>@@ -162,7 +159,7 @@ class PropertyCondition {
>     bool hasPrototype() const
>     {
>         return !!*this
>-            && (m_kind == Absence || m_kind == AbsenceOfSetEffect || m_kind == HasPrototype);
>+            && (m_header.type() == Absence || m_header.type() == AbsenceOfSetEffect || m_header.type() == HasPrototype);
>     }
>     JSObject* prototype() const
>     {
>@@ -170,7 +167,7 @@ class PropertyCondition {
>         return u.prototype.prototype;
>     }
>     
>-    bool hasRequiredValue() const { return !!*this && m_kind == Equivalence; }
>+    bool hasRequiredValue() const { return !!*this && m_header.type() == Equivalence; }
>     JSValue requiredValue() const
>     {
>         ASSERT(hasRequiredValue());
>@@ -182,8 +179,8 @@ class PropertyCondition {
>     
>     unsigned hash() const
>     {
>-        unsigned result = WTF::PtrHash<UniquedStringImpl*>::hash(m_uid) + static_cast<unsigned>(m_kind);
>-        switch (m_kind) {
>+        unsigned result = WTF::PtrHash<UniquedStringImpl*>::hash(m_header.pointer()) + static_cast<unsigned>(m_header.type());
>+        switch (m_header.type()) {
>         case Presence:
>             result ^= u.presence.offset;
>             result ^= u.presence.attributes;
>@@ -202,11 +199,11 @@ class PropertyCondition {
>     
>     bool operator==(const PropertyCondition& other) const
>     {
>-        if (m_uid != other.m_uid)
>+        if (m_header.pointer() != other.m_header.pointer())
>             return false;
>-        if (m_kind != other.m_kind)
>+        if (m_header.type() != other.m_header.type())
>             return false;
>-        switch (m_kind) {
>+        switch (m_header.type()) {
>         case Presence:
>             return u.presence.offset == other.u.presence.offset
>                 && u.presence.attributes == other.u.presence.attributes;
>@@ -223,7 +220,7 @@ class PropertyCondition {
>     
>     bool isHashTableDeletedValue() const
>     {
>-        return !m_uid && m_kind == Absence;
>+        return !m_header.pointer() && m_header.type() == Absence;
>     }
>     
>     // Two conditions are compatible if they are identical or if they speak of different uids. If
>@@ -296,7 +293,7 @@ class PropertyCondition {
>     }
>     bool watchingRequiresReplacementWatchpoint() const
>     {
>-        return !!*this && m_kind == Equivalence;
>+        return !!*this && m_header.type() == Equivalence;
>     }
>     
>     // This means that the objects involved in this are still live.
>@@ -313,8 +310,7 @@ class PropertyCondition {
> private:
>     bool isWatchableWhenValid(Structure*, WatchabilityEffort) const;
> 
>-    UniquedStringImpl* m_uid;
>-    Kind m_kind;
>+    Header m_header;
>     union {
>         struct {
>             PropertyOffset offset;
>@@ -322,8 +318,7 @@ class PropertyCondition {
>         } presence;
>         struct {
>             JSObject* prototype;
>-        } prototype;
>-        struct {
>+        } prototype; struct {
>             EncodedJSValue value;
>         } equivalence;
>     } u;
>diff --git a/Source/WTF/wtf/CMakeLists.txt b/Source/WTF/wtf/CMakeLists.txt
>index e46802a3444eb433022a59e58c678b1416d273b4..06b6201e2c2d145291d20712f2b5a9ccb0560a7a 100644
>--- a/Source/WTF/wtf/CMakeLists.txt
>+++ b/Source/WTF/wtf/CMakeLists.txt
>@@ -162,6 +162,7 @@ set(WTF_PUBLIC_HEADERS
>     PlatformRegisters.h
>     PointerComparison.h
>     PointerPreparations.h
>+    PointerWithType.h
>     Poisoned.h
>     PoisonedUniquePtr.h
>     PrintStream.h
>diff --git a/Source/WTF/wtf/PointerWithType.h b/Source/WTF/wtf/PointerWithType.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..31661395361da8bec21756af0dc23c4c6c804355
>--- /dev/null
>+++ b/Source/WTF/wtf/PointerWithType.h
>@@ -0,0 +1,83 @@
>+/*
>+ * Copyright (C) 2018 Yusuke Suzuki <utatane.tea@gmail.com>.
>+ *
>+ * 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
>+
>+#include <wtf/StdLibExtras.h>
>+
>+namespace WTF {
>+
>+// The goal of this class is folding a pointer and 1 byte value into 8 bytes in both 32bit and 64bit architectures.
>+// 32bit architecture just has a pair of byte and pointer, which should be 8 bytes. In 64bit, we use upper 16 bits.
>+
>+template<typename PointerType, typename Type>
>+class PointerWithType {
>+public:
>+    static_assert(sizeof(Type) == 1, "");
>+
>+    PointerWithType() = default;
>+
>+#if USE(JSVALUE64)
>+    PointerWithType(PointerType pointer, Type type)
>+        : m_data(bitwise_cast<uint64_t>(pointer) | (static_cast<uint64_t>(type) << 48))
>+    {
>+    }
>+
>+    PointerType pointer() const { return bitwise_cast<PointerType>(m_data & 0x0000FFFFFFFFFFFFULL); }
>+    void setPointer(PointerType pointer)
>+    {
>+        m_data = bitwise_cast<uint64_t>(pointer) | (m_data & 0xFFFF000000000000ULL);
>+    }
>+    Type type() const { return static_cast<Type>(m_data >> 48); }
>+    void setType(Type type)
>+    {
>+        m_data = (static_cast<uint64_t>(type) << 48) | (m_data & 0x0000FFFFFFFFFFFFULL);
>+    }
>+
>+#else
>+    PointerWithType(PointerType pointer, Type type)
>+        : m_pointer(pointer)
>+        , m_type(type)
>+    {
>+    }
>+
>+    PointerType pointer() const { return m_pointer; }
>+    void setPointer(PointerType pointer) { m_pointer = pointer; }
>+    Type type() const { return m_type; }
>+    void setType(Type type) { m_type = type; }
>+#endif
>+
>+private:
>+#if USE(JSVALUE64)
>+    uint64_t m_data { 0 };
>+#else
>+    PointerType m_pointer { nullptr };
>+    Type m_type { 0 };
>+#endif
>+};
>+
>+} // namespace WTF
>+
>+using WTF::PointerWithType;

Actions: View | Formatted Diff | Diff

Attachments on bug 188328: 346580 | 346586 | 346587 | 346588