Skip to content

Commit

Permalink
Deduplicate ImmutableStyleProperties
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259972
rdar://113622930

Reviewed by Alan Baradlay.

It is common for the same style declarations to repeat. On some sites >50% of declarations are duplicates.

This patch add support for hashing simple CSSValues. This mechanism may have
other uses in future too.

* Source/WebCore/css/CSSFontFaceSrcValue.cpp:
(WebCore::CSSFontFaceSrcResourceValue::equals const):

Also fix equality comparison here.

* Source/WebCore/css/CSSFunctionValue.cpp:
(WebCore::CSSFunctionValue::addDerivedHash const):
* Source/WebCore/css/CSSFunctionValue.h:
* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::addDerivedHash const):
* Source/WebCore/css/CSSPrimitiveValue.h:
* Source/WebCore/css/CSSValue.cpp:
(WebCore::CSSValue::visitDerived):
(WebCore::CSSValue::addHash const):
(WebCore::CSSValue::addDerivedHash const):
(WebCore::add):
* Source/WebCore/css/CSSValue.h:
* Source/WebCore/css/CSSValueList.cpp:
(WebCore::CSSValueContainingVector::addDerivedHash const):
* Source/WebCore/css/CSSValueList.h:
* Source/WebCore/css/CSSValuePair.cpp:
(WebCore::CSSValuePair::addDerivedHash const):
* Source/WebCore/css/CSSValuePair.h:
* Source/WebCore/css/ImmutableStyleProperties.cpp:
(WebCore::deduplicationMap):
(WebCore::ImmutableStyleProperties::createDeduplicating):

Deduplicate using a map.

(WebCore::ImmutableStyleProperties::clearDeduplicationMap):
* Source/WebCore/css/ImmutableStyleProperties.h:
* Source/WebCore/css/StyleProperties.cpp:
(WebCore::StyleProperties::immutableCopyIfNeeded const):
* Source/WebCore/css/parser/CSSParserImpl.cpp:
(WebCore::createStyleProperties):
* Source/WebCore/page/MemoryRelease.cpp:
(WebCore::releaseNoncriticalMemory):

Canonical link: https://commits.webkit.org/266770@main
  • Loading branch information
anttijk committed Aug 10, 2023
1 parent a7bd570 commit b94dfac
Show file tree
Hide file tree
Showing 16 changed files with 252 additions and 3 deletions.
5 changes: 4 additions & 1 deletion Source/WebCore/css/CSSFontFaceSrcValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ String CSSFontFaceSrcResourceValue::customCSSText() const

bool CSSFontFaceSrcResourceValue::equals(const CSSFontFaceSrcResourceValue& other) const
{
return m_location.specifiedURLString == m_location.specifiedURLString && m_format == other.m_format && m_technologies == other.m_technologies;
return m_location == other.m_location
&& m_format == other.m_format
&& m_technologies == other.m_technologies
&& m_loadedFromOpaqueSource == other.m_loadedFromOpaqueSource;
}

}
6 changes: 6 additions & 0 deletions Source/WebCore/css/CSSFunctionValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,10 @@ String CSSFunctionValue::customCSSText() const
return result.toString();
}

bool CSSFunctionValue::addDerivedHash(Hasher& hasher) const
{
add(hasher, m_name);
return CSSValueContainingVector::addDerivedHash(hasher);
}

}
4 changes: 4 additions & 0 deletions Source/WebCore/css/CSSFunctionValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,17 @@ class CSSFunctionValue final : public CSSValueContainingVector {
bool equals(const CSSFunctionValue& other) const { return m_name == other.m_name && itemsEqual(other); }

private:
friend bool CSSValue::addHash(Hasher&) const;

CSSFunctionValue(CSSValueID name, CSSValueListBuilder);
explicit CSSFunctionValue(CSSValueID name);
CSSFunctionValue(CSSValueID name, Ref<CSSValue>);
CSSFunctionValue(CSSValueID name, Ref<CSSValue>, Ref<CSSValue>);
CSSFunctionValue(CSSValueID name, Ref<CSSValue>, Ref<CSSValue>, Ref<CSSValue>);
CSSFunctionValue(CSSValueID name, Ref<CSSValue>, Ref<CSSValue>, Ref<CSSValue>, Ref<CSSValue>);

bool addDerivedHash(Hasher&) const;

CSSValueID m_name { };
};

Expand Down
104 changes: 104 additions & 0 deletions Source/WebCore/css/CSSPrimitiveValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "RenderBoxInlines.h"
#include "RenderStyle.h"
#include "RenderView.h"
#include <wtf/Hasher.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/StdLibExtras.h>
#include <wtf/text/StringBuilder.h>
Expand Down Expand Up @@ -1510,6 +1511,109 @@ bool CSSPrimitiveValue::equals(const CSSPrimitiveValue& other) const
return false;
}

bool CSSPrimitiveValue::addDerivedHash(Hasher& hasher) const
{
add(hasher, primitiveUnitType());

switch (primitiveUnitType()) {
case CSSUnitType::CSS_UNKNOWN:
break;
case CSSUnitType::CSS_NUMBER:
case CSSUnitType::CSS_INTEGER:
case CSSUnitType::CSS_PERCENTAGE:
case CSSUnitType::CSS_EMS:
case CSSUnitType::CSS_QUIRKY_EMS:
case CSSUnitType::CSS_EXS:
case CSSUnitType::CSS_REMS:
case CSSUnitType::CSS_CHS:
case CSSUnitType::CSS_IC:
case CSSUnitType::CSS_PX:
case CSSUnitType::CSS_CM:
case CSSUnitType::CSS_DPPX:
case CSSUnitType::CSS_X:
case CSSUnitType::CSS_DPI:
case CSSUnitType::CSS_DPCM:
case CSSUnitType::CSS_MM:
case CSSUnitType::CSS_IN:
case CSSUnitType::CSS_PT:
case CSSUnitType::CSS_PC:
case CSSUnitType::CSS_DEG:
case CSSUnitType::CSS_RAD:
case CSSUnitType::CSS_GRAD:
case CSSUnitType::CSS_MS:
case CSSUnitType::CSS_S:
case CSSUnitType::CSS_HZ:
case CSSUnitType::CSS_KHZ:
case CSSUnitType::CSS_TURN:
case CSSUnitType::CSS_VW:
case CSSUnitType::CSS_VH:
case CSSUnitType::CSS_VMIN:
case CSSUnitType::CSS_VMAX:
case CSSUnitType::CSS_VB:
case CSSUnitType::CSS_VI:
case CSSUnitType::CSS_SVW:
case CSSUnitType::CSS_SVH:
case CSSUnitType::CSS_SVMIN:
case CSSUnitType::CSS_SVMAX:
case CSSUnitType::CSS_SVB:
case CSSUnitType::CSS_SVI:
case CSSUnitType::CSS_LVW:
case CSSUnitType::CSS_LVH:
case CSSUnitType::CSS_LVMIN:
case CSSUnitType::CSS_LVMAX:
case CSSUnitType::CSS_LVB:
case CSSUnitType::CSS_LVI:
case CSSUnitType::CSS_DVW:
case CSSUnitType::CSS_DVH:
case CSSUnitType::CSS_DVMIN:
case CSSUnitType::CSS_DVMAX:
case CSSUnitType::CSS_DVB:
case CSSUnitType::CSS_DVI:
case CSSUnitType::CSS_FR:
case CSSUnitType::CSS_Q:
case CSSUnitType::CSS_LHS:
case CSSUnitType::CSS_RLHS:
case CSSUnitType::CSS_DIMENSION:
case CSSUnitType::CSS_CQW:
case CSSUnitType::CSS_CQH:
case CSSUnitType::CSS_CQI:
case CSSUnitType::CSS_CQB:
case CSSUnitType::CSS_CQMIN:
case CSSUnitType::CSS_CQMAX:
add(hasher, m_value.number);
break;
case CSSUnitType::CSS_PROPERTY_ID:
add(hasher, m_value.propertyID);
break;
case CSSUnitType::CSS_VALUE_ID:
add(hasher, m_value.valueID);
break;
case CSSUnitType::CSS_STRING:
case CSSUnitType::CustomIdent:
case CSSUnitType::CSS_URI:
case CSSUnitType::CSS_ATTR:
case CSSUnitType::CSS_COUNTER_NAME:
case CSSUnitType::CSS_FONT_FAMILY:
add(hasher, String { m_value.string });
break;
case CSSUnitType::CSS_RGBCOLOR:
add(hasher, color());
break;
case CSSUnitType::CSS_CALC:
add(hasher, m_value.calc);
break;
case CSSUnitType::CSS_UNRESOLVED_COLOR:
add(hasher, m_value.unresolvedColor);
break;
case CSSUnitType::CSS_IDENT:
case CSSUnitType::CSS_CALC_PERCENTAGE_WITH_NUMBER:
case CSSUnitType::CSS_CALC_PERCENTAGE_WITH_LENGTH:
ASSERT_NOT_REACHED();
return false;
}
return true;
}

// https://drafts.css-houdini.org/css-properties-values-api/#dependency-cycles
void CSSPrimitiveValue::collectComputedStyleDependencies(ComputedStyleDependencies& dependencies) const
{
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/css/CSSPrimitiveValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ class CSSPrimitiveValue final : public CSSValue {
friend class CSSValuePool;
friend class StaticCSSValuePool;
friend LazyNeverDestroyed<CSSPrimitiveValue>;
friend bool CSSValue::addHash(Hasher&) const;

explicit CSSPrimitiveValue(CSSPropertyID);
explicit CSSPrimitiveValue(Color);
Expand All @@ -219,6 +220,8 @@ class CSSPrimitiveValue final : public CSSValue {

double computeLengthDouble(const CSSToLengthConversionData&) const;

bool addDerivedHash(Hasher&) const;

ALWAYS_INLINE String serializeInternal() const;
NEVER_INLINE String formatNumberValue(ASCIILiteral suffix) const;
NEVER_INLINE String formatIntegerValue(ASCIILiteral suffix) const;
Expand Down Expand Up @@ -430,6 +433,8 @@ inline int CSSValue::integer() const
return downcast<CSSPrimitiveValue>(*this).intValue();
}

void add(Hasher&, const CSSPrimitiveValue&);

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_CSS_VALUE(CSSPrimitiveValue, isPrimitiveValue())
25 changes: 25 additions & 0 deletions Source/WebCore/css/CSSValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
#include "DeprecatedCSSOMPrimitiveValue.h"
#include "DeprecatedCSSOMValueList.h"
#include "EventTarget.h"
#include <wtf/Hasher.h>

namespace WebCore {

Expand Down Expand Up @@ -281,6 +282,25 @@ bool CSSValue::equals(const CSSValue& other) const
return false;
}

bool CSSValue::addHash(Hasher& hasher) const
{
// To match equals() a single item list could have the same hash as the item.
// FIXME: Some Style::Builder functions can only handle list values.

add(hasher, classType());

return visitDerived([&](auto& typedThis) {
return typedThis.addDerivedHash(hasher);
});
}

// FIXME: Add custom hash functions for all derived classes and remove this function.
bool CSSValue::addDerivedHash(Hasher& hasher) const
{
add(hasher, this);
return false;
}

bool CSSValue::isCSSLocalURL(StringView relativeURL)
{
return relativeURL.isEmpty() || relativeURL.startsWith('#');
Expand Down Expand Up @@ -339,4 +359,9 @@ Ref<DeprecatedCSSOMValue> CSSValue::createDeprecatedCSSOMWrapper(CSSStyleDeclara
}
}

void add(Hasher& hasher, const CSSValue& value)
{
value.addHash(hasher);
}

}
10 changes: 10 additions & 0 deletions Source/WebCore/css/CSSValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
#include <wtf/Vector.h>
#include <wtf/text/ASCIILiteral.h>

namespace WTF {
class Hasher;
}

namespace WebCore {

class CSSPrimitiveValue;
Expand Down Expand Up @@ -151,6 +155,9 @@ class CSSValue {
bool equals(const CSSValue&) const;
bool operator==(const CSSValue& other) const { return equals(other); }

// Returns false if the hash is computed from the CSSValue pointer instead of the underlying values.
bool addHash(Hasher&) const;

// https://www.w3.org/TR/css-values-4/#local-urls
// Empty URLs and fragment-only URLs should not be resolved relative to the base URL.
static bool isCSSLocalURL(StringView relativeURL);
Expand Down Expand Up @@ -286,6 +293,7 @@ class CSSValue {
template<typename Visitor> constexpr decltype(auto) visitDerived(Visitor&&) const;

static inline bool customTraverseSubresources(const Function<bool(const CachedResource&)>&);
bool addDerivedHash(Hasher&) const;

mutable unsigned m_refCount { refCountIncrement };

Expand Down Expand Up @@ -347,6 +355,8 @@ inline bool compareCSSValue(const Ref<CSSValueType>& first, const Ref<CSSValueTy
return first.get().equals(second);
}

void add(Hasher&, const CSSValue&);

} // namespace WebCore

#define SPECIALIZE_TYPE_TRAITS_CSS_VALUE(ToValueTypeName, predicate) \
Expand Down
12 changes: 12 additions & 0 deletions Source/WebCore/css/CSSValueList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "CSSValueList.h"

#include "CSSPrimitiveValue.h"
#include <wtf/Hasher.h>
#include <wtf/text/StringBuilder.h>

namespace WebCore {
Expand Down Expand Up @@ -259,6 +260,17 @@ bool CSSValueContainingVector::containsSingleEqualItem(const CSSValue& other) co
return size() == 1 && (*this)[0].equals(other);
}

bool CSSValueContainingVector::addDerivedHash(Hasher& hasher) const
{
add(hasher, separator());

for (auto& item : *this) {
if (!item.addHash(hasher))
return false;
}
return true;
}

bool CSSValueContainingVector::customTraverseSubresources(const Function<bool(const CachedResource&)>& handler) const
{
for (auto& value : *this) {
Expand Down
8 changes: 8 additions & 0 deletions Source/WebCore/css/CSSValueList.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class CSSValueContainingVector : public CSSValue {
const CSSValue* itemWithoutBoundsCheck(unsigned index) const { return &(*this)[index]; }

protected:
friend bool CSSValue::addHash(Hasher&) const;

CSSValueContainingVector(ClassType, ValueSeparator);
CSSValueContainingVector(ClassType, ValueSeparator, CSSValueListBuilder);
CSSValueContainingVector(ClassType, ValueSeparator, Ref<CSSValue>);
Expand All @@ -82,6 +84,8 @@ class CSSValueContainingVector : public CSSValue {
CSSValueContainingVector(ClassType, ValueSeparator, Ref<CSSValue>, Ref<CSSValue>, Ref<CSSValue>, Ref<CSSValue>);
~CSSValueContainingVector();

bool addDerivedHash(Hasher&) const;

private:
unsigned m_size { 0 };
std::array<const CSSValue*, 4> m_inlineStorage;
Expand Down Expand Up @@ -110,6 +114,8 @@ class CSSValueList : public CSSValueContainingVector {
bool equals(const CSSValueList&) const;

private:
friend void add(Hasher&, const CSSValueList&);

explicit CSSValueList(ValueSeparator);
CSSValueList(ValueSeparator, CSSValueListBuilder);
CSSValueList(ValueSeparator, Ref<CSSValue>);
Expand All @@ -135,6 +141,8 @@ inline const CSSValue& CSSValueContainingVector::operator[](unsigned index) cons
return *m_additionalStorage[index - maxInlineSize];
}

void add(Hasher&, const CSSValueContainingVector&);

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_CSS_VALUE(CSSValueContainingVector, containsVector())
Expand Down
7 changes: 7 additions & 0 deletions Source/WebCore/css/CSSValuePair.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "config.h"
#include "CSSValuePair.h"
#include <wtf/Hasher.h>
#include <wtf/text/WTFString.h>

namespace WebCore {
Expand Down Expand Up @@ -64,4 +65,10 @@ bool CSSValuePair::equals(const CSSValuePair& other) const
&& m_second->equals(other.m_second);
}

bool CSSValuePair::addDerivedHash(Hasher& hasher) const
{
add(hasher, m_valueSeparator, m_coalesceIdenticalValues);
return m_first->addHash(hasher) && m_second->addHash(hasher);
}

} // namespace WebCore
4 changes: 4 additions & 0 deletions Source/WebCore/css/CSSValuePair.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,13 @@ class CSSValuePair : public CSSValue {
bool equals(const CSSValuePair&) const;

private:
friend bool CSSValue::addHash(Hasher&) const;

enum class IdenticalValueSerialization : bool { DoNotCoalesce, Coalesce };
CSSValuePair(Ref<CSSValue>, Ref<CSSValue>, IdenticalValueSerialization);

bool addDerivedHash(Hasher&) const;

// FIXME: Store coalesce bit in CSSValue to cut down on object size.
bool m_coalesceIdenticalValues { true };
Ref<CSSValue> m_first;
Expand Down
Loading

0 comments on commit b94dfac

Please sign in to comment.