Changeset 256494 in webkit


Ignore:
Timestamp:
Feb 12, 2020 5:11:08 PM (4 years ago)
Author:
ysuzuki@apple.com
Message:

CSSValuePool's constant CSS values should not be allocated dynamically (and same for Vectors)
​https://bugs.webkit.org/show_bug.cgi?id=207666

Reviewed by Mark Lam.

r252785 changes contents (CSSValues and Vectors) of CSSValuePool from static ones to
dynamically allocated ones. This was done since we would like to use static CSSValues
even in the other threads (workers etc.) for OffscreenCanvas feature.

But this causes memory regression in Membuster since we allocates many CSSValues and
large Vectors, and they are kept persistently.

This patch removes dynamic allocation part of r252785 to recover memory regression.
The key of this patch is introducing Static CSSValue feature. When incrementing / decrementing
m_refCount of CSSValue, we add / subtract by 0x2. And we put 0x1 as a static-flag. So, even if
this CSSValue is used by multiple threads, we never see that CSSValue gets 0 m_refCount if
it is marked as static (having 0x1). This is the same design to our static StringImpl.

No behavior change.

  • css/CSSInheritedValue.h:
  • css/CSSInitialValue.h:
  • css/CSSPrimitiveValue.cpp:

(WebCore::CSSPrimitiveValue::CSSPrimitiveValue):

  • css/CSSPrimitiveValue.h:
  • css/CSSRevertValue.h:
  • css/CSSUnsetValue.h:
  • css/CSSValue.cpp:
  • css/CSSValue.h:

(WebCore::CSSValue::ref const):
(WebCore::CSSValue::hasOneRef const):
(WebCore::CSSValue::refCount const):
(WebCore::CSSValue::hasAtLeastOneRef const):
(WebCore::CSSValue::deref):
(WebCore::CSSValue::makeStatic):

  • css/CSSValuePool.cpp:

(WebCore::StaticCSSValuePool::StaticCSSValuePool):
(WebCore::StaticCSSValuePool::init):
(WebCore::CSSValuePool::CSSValuePool):
(WebCore::CSSValuePool::singleton):
(WebCore::CSSValuePool::createIdentifierValue):
(WebCore::CSSValuePool::createColorValue):
(WebCore::CSSValuePool::createValue):

  • css/CSSValuePool.h:

(WebCore::CSSValuePool::createInheritedValue):
(WebCore::CSSValuePool::createImplicitInitialValue):
(WebCore::CSSValuePool::createExplicitInitialValue):
(WebCore::CSSValuePool::createUnsetValue):
(WebCore::CSSValuePool::createRevertValue):

Location:
trunk/Source/WebCore
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r256493 r256494  
     12020-02-12  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        CSSValuePool's constant CSS values should not be allocated dynamically (and same for Vectors)
     4        https://bugs.webkit.org/show_bug.cgi?id=207666
     5
     6        Reviewed by Mark Lam.
     7
     8        r252785 changes contents (CSSValues and Vectors) of CSSValuePool from static ones to
     9        dynamically allocated ones. This was done since we would like to use static CSSValues
     10        even in the other threads (workers etc.) for OffscreenCanvas feature.
     11
     12        But this causes memory regression in Membuster since we allocates many CSSValues and
     13        large Vectors, and they are kept persistently.
     14
     15        This patch removes dynamic allocation part of r252785 to recover memory regression.
     16        The key of this patch is introducing Static CSSValue feature. When incrementing / decrementing
     17        m_refCount of CSSValue, we add / subtract by 0x2. And we put 0x1 as a static-flag. So, even if
     18        this CSSValue is used by multiple threads, we never see that CSSValue gets 0 m_refCount if
     19        it is marked as static (having 0x1). This is the same design to our static StringImpl.
     20
     21        No behavior change.
     22
     23        * css/CSSInheritedValue.h:
     24        * css/CSSInitialValue.h:
     25        * css/CSSPrimitiveValue.cpp:
     26        (WebCore::CSSPrimitiveValue::CSSPrimitiveValue):
     27        * css/CSSPrimitiveValue.h:
     28        * css/CSSRevertValue.h:
     29        * css/CSSUnsetValue.h:
     30        * css/CSSValue.cpp:
     31        * css/CSSValue.h:
     32        (WebCore::CSSValue::ref const):
     33        (WebCore::CSSValue::hasOneRef const):
     34        (WebCore::CSSValue::refCount const):
     35        (WebCore::CSSValue::hasAtLeastOneRef const):
     36        (WebCore::CSSValue::deref):
     37        (WebCore::CSSValue::makeStatic):
     38        * css/CSSValuePool.cpp:
     39        (WebCore::StaticCSSValuePool::StaticCSSValuePool):
     40        (WebCore::StaticCSSValuePool::init):
     41        (WebCore::CSSValuePool::CSSValuePool):
     42        (WebCore::CSSValuePool::singleton):
     43        (WebCore::CSSValuePool::createIdentifierValue):
     44        (WebCore::CSSValuePool::createColorValue):
     45        (WebCore::CSSValuePool::createValue):
     46        * css/CSSValuePool.h:
     47        (WebCore::CSSValuePool::createInheritedValue):
     48        (WebCore::CSSValuePool::createImplicitInitialValue):
     49        (WebCore::CSSValuePool::createExplicitInitialValue):
     50        (WebCore::CSSValuePool::createUnsetValue):
     51        (WebCore::CSSValuePool::createRevertValue):
     52
    1532020-02-12  Ryan Haddad  <ryanhaddad@apple.com>
    254

  • trunk/Source/WebCore/css/CSSInheritedValue.h

    r252785 r256494  
    2727class CSSInheritedValue final : public CSSValue {
    2828public:
    29     static Ref<CSSInheritedValue> create() { return adoptRef(*new CSSInheritedValue()); }
    30 
    3129    String customCSSText() const;
    3230
    … …  
    3432
    3533private:
    36     CSSInheritedValue()
     34    friend LazyNeverDestroyed<CSSInheritedValue>;
     35    CSSInheritedValue(StaticCSSValueTag)
    3736        : CSSValue(InheritedClass)
    3837    {
     38        makeStatic();
    3939    }
    4040};

  • trunk/Source/WebCore/css/CSSInitialValue.h

    r235986 r256494  
    2727class CSSInitialValue final : public CSSValue {
    2828public:
    29     static Ref<CSSInitialValue> createExplicit()
    30     {
    31         return adoptRef(*new CSSInitialValue(/* implicit */ false));
    32     }
    33     static Ref<CSSInitialValue> createImplicit()
    34     {
    35         return adoptRef(*new CSSInitialValue(/* implicit */ true));
    36     }
    37 
    3829    String customCSSText() const;
    3930
    … …  
    4435private:
    4536    friend LazyNeverDestroyed<CSSInitialValue>;
    46     CSSInitialValue(bool implicit)
     37    CSSInitialValue(StaticCSSValueTag, bool implicit)
    4738        : CSSValue(InitialClass)
    4839        , m_isImplicit(implicit)
    4940    {
     41        makeStatic();
    5042    }
    5143

  • trunk/Source/WebCore/css/CSSPrimitiveValue.cpp

    r255785 r256494  
    305305{
    306306    init(lengthSize, style);
     307}
     308
     309CSSPrimitiveValue::CSSPrimitiveValue(StaticCSSValueTag, CSSValueID valueID)
     310    : CSSPrimitiveValue(valueID)
     311{
     312    makeStatic();
     313}
     314
     315CSSPrimitiveValue::CSSPrimitiveValue(StaticCSSValueTag, const Color& color)
     316    : CSSPrimitiveValue(color)
     317{
     318    makeStatic();
     319}
     320
     321CSSPrimitiveValue::CSSPrimitiveValue(StaticCSSValueTag, double num, CSSUnitType type)
     322    : CSSPrimitiveValue(num, type)
     323{
     324    makeStatic();
    307325}
    308326

  • trunk/Source/WebCore/css/CSSPrimitiveValue.h

    r255785 r256494  
    219219    CSSPrimitiveValue(double, CSSUnitType);
    220220
     221    CSSPrimitiveValue(StaticCSSValueTag, CSSValueID);
     222    CSSPrimitiveValue(StaticCSSValueTag, const Color&);
     223    CSSPrimitiveValue(StaticCSSValueTag, double, CSSUnitType);
     224
    221225    template<typename T> CSSPrimitiveValue(T); // Defined in CSSPrimitiveValueMappings.h
    222226    template<typename T> CSSPrimitiveValue(RefPtr<T>&&);

  • trunk/Source/WebCore/css/CSSRevertValue.h

    r252785 r256494  
    3232class CSSRevertValue final : public CSSValue {
    3333public:
    34     static Ref<CSSRevertValue> create() { return adoptRef(*new CSSRevertValue()); }
    35 
    3634    String customCSSText() const;
    3735
    … …  
    3937
    4038private:
    41     CSSRevertValue()
     39    friend LazyNeverDestroyed<CSSRevertValue>;
     40    CSSRevertValue(StaticCSSValueTag)
    4241        : CSSValue(RevertClass)
    4342    {
     43        makeStatic();
    4444    }
    4545};

  • trunk/Source/WebCore/css/CSSUnsetValue.h

    r252785 r256494  
    3232class CSSUnsetValue final : public CSSValue {
    3333public:
    34     static Ref<CSSUnsetValue> create() { return adoptRef(*new CSSUnsetValue()); }
    35 
    3634    String customCSSText() const;
    3735
    … …  
    3937
    4038private:
    41     CSSUnsetValue()
     39    friend LazyNeverDestroyed<CSSUnsetValue>;
     40    CSSUnsetValue(StaticCSSValueTag)
    4241        : CSSValue(UnsetClass)
    4342    {
     43        makeStatic();
    4444    }
    4545};

  • trunk/Source/WebCore/css/CSSValue.cpp

    r253987 r256494  
    7575namespace WebCore {
    7676
    77 struct SameSizeAsCSSValue : public RefCounted<SameSizeAsCSSValue> {
     77struct SameSizeAsCSSValue {
     78    uint32_t refCount;
    7879    uint32_t bitfields;
    7980};

  • trunk/Source/WebCore/css/CSSValue.h

    r253987 r256494  
    4040
    4141DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(CSSValue);
    42 class CSSValue : public RefCounted<CSSValue> {
     42class CSSValue {
     43    WTF_MAKE_NONCOPYABLE(CSSValue);
    4344    WTF_MAKE_FAST_ALLOCATED_WITH_HEAP_IDENTIFIER(CSSValue);
    4445public:
    … …  
    5354    };
    5455
    55     // Override RefCounted's deref() to ensure operator delete is called on
    56     // the appropriate subclass type.
     56    static constexpr unsigned refCountFlagIsStatic = 0x1;
     57    static constexpr unsigned refCountIncrement = 0x2; // This allows us to ref / deref without disturbing the static CSSValue flag.
     58    void ref() const
     59    {
     60        m_refCount += refCountIncrement;
     61    }
     62    bool hasOneRef() const { return m_refCount == refCountIncrement; }
     63    unsigned refCount() const { return m_refCount / refCountIncrement; }
     64    bool hasAtLeastOneRef() const { return m_refCount; }
     65
    5766    void deref()
    5867    {
    59         if (derefBase())
     68        // Customized deref() to ensure operator delete is called on
     69        // the appropriate subclass type.
     70        unsigned tempRefCount = m_refCount - refCountIncrement;
     71        if (!tempRefCount) {
    6072            destroy();
     73            return;
     74        }
     75        m_refCount = tempRefCount;
    6176    }
    6277
    … …  
    215230        SlashSeparator
    216231    };
     232    enum StaticCSSValueTag { StaticCSSValue };
    217233
    218234protected:
    … …  
    228244    }
    229245
     246    void makeStatic()
     247    {
     248        m_refCount |= refCountFlagIsStatic;
     249    }
     250
    230251    // NOTE: This class is non-virtual for memory and performance reasons.
    231252    // Don't go making it virtual again unless you know exactly what you're doing!
    … …  
    236257    WEBCORE_EXPORT void destroy();
    237258
     259    mutable unsigned m_refCount { refCountIncrement };
    238260protected:
    239261    // The bits in this section are only used by specific subclasses but kept here
    240262    // to maximize struct packing.
    241 
    242263    // CSSPrimitiveValue bits:
    243264    unsigned m_primitiveUnitType : 7; // CSSUnitType

  • trunk/Source/WebCore/css/CSSValuePool.cpp

    r254703 r256494  
    3535namespace WebCore {
    3636
     37LazyNeverDestroyed<StaticCSSValuePool> staticCSSValuePool;
     38
     39StaticCSSValuePool::StaticCSSValuePool()
     40{
     41    m_inheritedValue.construct(CSSValue::StaticCSSValue);
     42    m_implicitInitialValue.construct(CSSValue::StaticCSSValue, true);
     43    m_explicitInitialValue.construct(CSSValue::StaticCSSValue, false);
     44    m_unsetValue.construct(CSSValue::StaticCSSValue);
     45    m_revertValue.construct(CSSValue::StaticCSSValue);
     46
     47    m_transparentColor.construct(CSSValue::StaticCSSValue, Color(Color::transparent));
     48    m_whiteColor.construct(CSSValue::StaticCSSValue, Color(Color::white));
     49    m_blackColor.construct(CSSValue::StaticCSSValue, Color(Color::black));
     50
     51    for (unsigned i = firstCSSValueKeyword; i <= lastCSSValueKeyword; ++i)
     52        m_identifierValues[i].construct(CSSValue::StaticCSSValue, static_cast<CSSValueID>(i));
     53
     54    for (unsigned i = 0; i < (maximumCacheableIntegerValue + 1); ++i) {
     55        m_pixelValues[i].construct(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_PX);
     56        m_percentValues[i].construct(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_PERCENTAGE);
     57        m_numberValues[i].construct(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_NUMBER);
     58    }
     59}
     60
     61void StaticCSSValuePool::init()
     62{
     63    static std::once_flag onceKey;
     64    std::call_once(onceKey, []() {
     65        staticCSSValuePool.construct();
     66    });
     67}
     68
     69CSSValuePool::CSSValuePool()
     70{
     71    StaticCSSValuePool::init();
     72}
     73
    3774CSSValuePool& CSSValuePool::singleton()
    3875{
    … …  
    4279}
    4380
    44 CSSValuePool::CSSValuePool()
    45     : m_inheritedValue(CSSInheritedValue::create())
    46     , m_implicitInitialValue(CSSInitialValue::createImplicit())
    47     , m_explicitInitialValue(CSSInitialValue::createExplicit())
    48     , m_unsetValue(CSSUnsetValue::create())
    49     , m_revertValue(CSSRevertValue::create())
    50     , m_transparentColor(CSSPrimitiveValue::create(Color(Color::transparent)))
    51     , m_whiteColor(CSSPrimitiveValue::create(Color(Color::white)))
    52     , m_blackColor(CSSPrimitiveValue::create(Color(Color::black)))
    53 {
    54     m_identifierValues.reserveInitialCapacity(numCSSValueKeywords);
    55     for (unsigned i = 0; i < numCSSValueKeywords; ++i)
    56         m_identifierValues.uncheckedAppend(CSSPrimitiveValue::create(static_cast<CSSValueID>(i)));
    57 
    58     m_pixelValues.reserveInitialCapacity(maximumCacheableIntegerValue + 1);
    59     m_percentValues.reserveInitialCapacity(maximumCacheableIntegerValue + 1);
    60     m_numberValues.reserveInitialCapacity(maximumCacheableIntegerValue + 1);
    61     for (unsigned i = 0; i < (maximumCacheableIntegerValue + 1); ++i) {
    62         m_pixelValues.uncheckedAppend(CSSPrimitiveValue::create(i, CSSUnitType::CSS_PX));
    63         m_percentValues.uncheckedAppend(CSSPrimitiveValue::create(i, CSSUnitType::CSS_PERCENTAGE));
    64         m_numberValues.uncheckedAppend(CSSPrimitiveValue::create(i, CSSUnitType::CSS_NUMBER));
    65     }
    66 }
    67 
    6881Ref<CSSPrimitiveValue> CSSValuePool::createIdentifierValue(CSSValueID ident)
    6982{
    7083    RELEASE_ASSERT(ident >= firstCSSValueKeyword && ident <= lastCSSValueKeyword);
    71     return m_identifierValues[ident].get();
     84    return staticCSSValuePool->m_identifierValues[ident].get();
    7285}
    7386
    … …  
    8194    // These are the empty and deleted values of the hash table.
    8295    if (color == Color::transparent)
    83         return m_transparentColor.get();
     96        return staticCSSValuePool->m_transparentColor.get();
    8497    if (color == Color::white)
    85         return m_whiteColor.get();
     98        return staticCSSValuePool->m_whiteColor.get();
    8699    // Just because it is common.
    87100    if (color == Color::black)
    88         return m_blackColor.get();
     101        return staticCSSValuePool->m_blackColor.get();
    89102
    90103    // Remove one entry at random if the cache grows too large.
    … …  
    103116    ASSERT(std::isfinite(value));
    104117
    105     if (value < 0 || value > maximumCacheableIntegerValue)
     118    if (value < 0 || value > StaticCSSValuePool::maximumCacheableIntegerValue)
    106119        return CSSPrimitiveValue::create(value, type);
    107120
    … …  
    112125    switch (type) {
    113126    case CSSUnitType::CSS_PX:
    114         return m_pixelValues[intValue].get();
     127        return staticCSSValuePool->m_pixelValues[intValue].get();
    115128    case CSSUnitType::CSS_PERCENTAGE:
    116         return m_percentValues[intValue].get();
     129        return staticCSSValuePool->m_percentValues[intValue].get();
    117130    case CSSUnitType::CSS_NUMBER:
    118         return m_numberValues[intValue].get();
     131        return staticCSSValuePool->m_numberValues[intValue].get();
    119132    default:
    120133        return CSSPrimitiveValue::create(value, type);

  • trunk/Source/WebCore/css/CSSValuePool.h

    r252785 r256494  
    3838#include <wtf/NeverDestroyed.h>
    3939#include <wtf/RefPtr.h>
    40 #include <wtf/Vector.h>
    4140#include <wtf/text/AtomStringHash.h>
    4241
    … …  
    4443
    4544class CSSValueList;
     45class CSSValuePool;
    4646
    4747enum class FromSystemFontID { No, Yes };
     48
     49class StaticCSSValuePool {
     50    friend class CSSValuePool;
     51    friend class LazyNeverDestroyed<StaticCSSValuePool>;
     52public:
     53    static void init();
     54
     55private:
     56    StaticCSSValuePool();
     57
     58    LazyNeverDestroyed<CSSInheritedValue> m_inheritedValue;
     59    LazyNeverDestroyed<CSSInitialValue> m_implicitInitialValue;
     60    LazyNeverDestroyed<CSSInitialValue> m_explicitInitialValue;
     61    LazyNeverDestroyed<CSSUnsetValue> m_unsetValue;
     62    LazyNeverDestroyed<CSSRevertValue> m_revertValue;
     63
     64    LazyNeverDestroyed<CSSPrimitiveValue> m_transparentColor;
     65    LazyNeverDestroyed<CSSPrimitiveValue> m_whiteColor;
     66    LazyNeverDestroyed<CSSPrimitiveValue> m_blackColor;
     67
     68    static constexpr int maximumCacheableIntegerValue = 255;
     69
     70    LazyNeverDestroyed<CSSPrimitiveValue> m_pixelValues[maximumCacheableIntegerValue + 1];
     71    LazyNeverDestroyed<CSSPrimitiveValue> m_percentValues[maximumCacheableIntegerValue + 1];
     72    LazyNeverDestroyed<CSSPrimitiveValue> m_numberValues[maximumCacheableIntegerValue + 1];
     73    LazyNeverDestroyed<CSSPrimitiveValue> m_identifierValues[numCSSValueKeywords];
     74};
     75
     76WEBCORE_EXPORT extern LazyNeverDestroyed<StaticCSSValuePool> staticCSSValuePool;
    4877
    4978class CSSValuePool {
    … …  
    5685    RefPtr<CSSValueList> createFontFaceValue(const AtomString&);
    5786    Ref<CSSPrimitiveValue> createFontFamilyValue(const String&, FromSystemFontID = FromSystemFontID::No);
    58     Ref<CSSInheritedValue> createInheritedValue() { return m_inheritedValue.get(); }
    59     Ref<CSSInitialValue> createImplicitInitialValue() { return m_implicitInitialValue.get(); }
    60     Ref<CSSInitialValue> createExplicitInitialValue() { return m_explicitInitialValue.get(); }
    61     Ref<CSSUnsetValue> createUnsetValue() { return m_unsetValue.get(); }
    62     Ref<CSSRevertValue> createRevertValue() { return m_revertValue.get(); }
     87    Ref<CSSInheritedValue> createInheritedValue() { return staticCSSValuePool->m_inheritedValue.get(); }
     88    Ref<CSSInitialValue> createImplicitInitialValue() { return staticCSSValuePool->m_implicitInitialValue.get(); }
     89    Ref<CSSInitialValue> createExplicitInitialValue() { return staticCSSValuePool->m_explicitInitialValue.get(); }
     90    Ref<CSSUnsetValue> createUnsetValue() { return staticCSSValuePool->m_unsetValue.get(); }
     91    Ref<CSSRevertValue> createRevertValue() { return staticCSSValuePool->m_revertValue.get(); }
    6392    Ref<CSSPrimitiveValue> createIdentifierValue(CSSValueID identifier);
    6493    Ref<CSSPrimitiveValue> createIdentifierValue(CSSPropertyID identifier);
    … …  
    82111    FontFamilyValueCache m_fontFamilyValueCache;
    83112
    84     friend class WTF::NeverDestroyed<CSSValuePool>;
    85 
    86     Ref<CSSInheritedValue> m_inheritedValue;
    87     Ref<CSSInitialValue> m_implicitInitialValue;
    88     Ref<CSSInitialValue> m_explicitInitialValue;
    89     Ref<CSSUnsetValue> m_unsetValue;
    90     Ref<CSSRevertValue> m_revertValue;
    91 
    92     Ref<CSSPrimitiveValue> m_transparentColor;
    93     Ref<CSSPrimitiveValue> m_whiteColor;
    94     Ref<CSSPrimitiveValue> m_blackColor;
    95 
    96     static const int maximumCacheableIntegerValue = 255;
    97 
    98     Vector<Ref<CSSPrimitiveValue>> m_pixelValues;
    99     Vector<Ref<CSSPrimitiveValue>> m_percentValues;
    100     Vector<Ref<CSSPrimitiveValue>> m_numberValues;
    101     Vector<Ref<CSSPrimitiveValue>> m_identifierValues;
     113    friend class NeverDestroyed<CSSValuePool>;
    102114};
    103115
Note: See TracChangeset for help on using the changeset viewer.