Skip to content

Commit

Permalink
[CSSTransition] Do not list up all properties for transition: all
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261917
rdar://115861030

Reviewed by Antti Koivisto.

When `transition: all ...` CSS rule is applied, we end up listing all animatable properties.
This is incredibly costly since there are so many CSS properties. And all properties are queried,
checked equality via very slow virtual function. Many hash table lookups etc.
We found that we are using massive amount of time in this transitioning in Speedometer3/NewsSite-Next / NewsSite-Nuxt.

But in almost all cases, almost all properties are not related at all.

In this patch, we do RenderStyle comparison and collect all affected CSS properties candidate which needs to be visited in
updateCSSTransitions. We can conservatively say "this property needs to be visited". But if we know that this
is totally equal, then we can skip that.

We cannot use RenderStyle::diff for this purpose. It is highly tailored to rendering purpose. And a lof of
properties changes are not reported when it is unrelated to layout (but it is necessary for updateCSSTransitions).
Example is `top` / `left`. So instead, we have more tailored path collecting necessary properties in this patch.

With this patch, we always collect all possible affected properties. So we no longer need to list up all animatable properties.
It results in 0.7% progression in Speedometer3.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::hasAnimations const):
* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/ElementRareData.h:
(WebCore::ElementRareData::hasAnimations const):
* Source/WebCore/style/Styleable.cpp:
(WebCore::Styleable::updateCSSTransitions const):

Canonical link: https://commits.webkit.org/268412@main
  • Loading branch information
Constellation committed Sep 25, 2023
1 parent abab8aa commit 66eb8a0
Show file tree
Hide file tree
Showing 11 changed files with 926 additions and 34 deletions.
2 changes: 0 additions & 2 deletions Source/WebCore/animation/CSSPropertyAnimation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3777,7 +3777,6 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
new DiscretePropertyWrapper<PrintColorAdjust>(CSSPropertyPrintColorAdjust, &RenderStyle::printColorAdjust, &RenderStyle::setPrintColorAdjust),
new DiscretePropertyWrapper<ColumnFill>(CSSPropertyColumnFill, &RenderStyle::columnFill, &RenderStyle::setColumnFill),
new DiscretePropertyWrapper<BorderStyle>(CSSPropertyColumnRuleStyle, &RenderStyle::columnRuleStyle, &RenderStyle::setColumnRuleStyle),
new DiscretePropertyWrapper<BorderStyle>(CSSPropertyColumnRuleStyle, &RenderStyle::columnRuleStyle, &RenderStyle::setColumnRuleStyle),
new DiscretePropertyWrapper<CursorType>(CSSPropertyCursor, &RenderStyle::cursor, &RenderStyle::setCursor),
new DiscretePropertyWrapper<EmptyCell>(CSSPropertyEmptyCells, &RenderStyle::emptyCells, &RenderStyle::setEmptyCells),
new DiscretePropertyWrapper<FlexDirection>(CSSPropertyFlexDirection, &RenderStyle::flexDirection, &RenderStyle::setFlexDirection),
Expand Down Expand Up @@ -3857,7 +3856,6 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
new OffsetRotateWrapper,

new PropertyWrapperContent,
new OffsetRotateWrapper,
new DiscretePropertyWrapper<TextDecorationSkipInk>(CSSPropertyTextDecorationSkipInk, &RenderStyle::textDecorationSkipInk, &RenderStyle::setTextDecorationSkipInk),
new DiscreteSVGPropertyWrapper<ColorInterpolation>(CSSPropertyColorInterpolation, &SVGRenderStyle::colorInterpolation, &SVGRenderStyle::setColorInterpolation),
new DiscreteFontDescriptionTypedWrapper<Kerning>(CSSPropertyFontKerning, &FontCascadeDescription::kerning, &FontCascadeDescription::setKerning),
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/animation/WebAnimationTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "CSSPropertyNames.h"
#include "CSSValue.h"
#include <wtf/BitSet.h>
#include <wtf/HashMap.h>
#include <wtf/ListHashSet.h>
#include <wtf/Markable.h>
Expand Down Expand Up @@ -71,6 +72,10 @@ using CSSAnimationCollection = ListHashSet<Ref<CSSAnimation>>;
using AnimatableProperty = std::variant<CSSPropertyID, AtomString>;
using AnimatablePropertyToTransitionMap = HashMap<AnimatableProperty, Ref<CSSTransition>>;

struct CSSPropertiesBitSet {
WTF::BitSet<numCSSProperties> m_properties { };
};

} // namespace WebCore

namespace WTF {
Expand Down
9 changes: 8 additions & 1 deletion Source/WebCore/css/CSSProperty.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "CSSPropertyNames.h"
#include "CSSValue.h"
#include "WritingMode.h"
#include <wtf/BitSet.h>
#include <wtf/RefPtr.h>

namespace WebCore {
Expand Down Expand Up @@ -78,12 +79,18 @@ class CSSProperty {
static bool isInLogicalPropertyGroup(CSSPropertyID);
static bool areInSameLogicalPropertyGroupWithDifferentMappingLogic(CSSPropertyID, CSSPropertyID);
static bool isDescriptorOnly(CSSPropertyID);
static bool isColorProperty(CSSPropertyID);
static UChar listValuedPropertySeparator(CSSPropertyID);
static bool isListValuedProperty(CSSPropertyID propertyID) { return !!listValuedPropertySeparator(propertyID); }
static bool allowsNumberOrIntegerInput(CSSPropertyID);

const StylePropertyMetadata& metadata() const { return m_metadata; }
static bool isColorProperty(CSSPropertyID propertyId)
{
return colorProperties.get(propertyId);
}

static const WEBCORE_EXPORT WTF::BitSet<numCSSProperties> colorProperties;
static const WEBCORE_EXPORT WTF::BitSet<numCSSProperties> physicalProperties;

bool operator==(const CSSProperty& other) const
{
Expand Down
29 changes: 27 additions & 2 deletions Source/WebCore/css/process-css-properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -2411,6 +2411,18 @@ def generate_property_id_switch_function_bool(self, *, to, signature, iterable,
to.write(f"}}")
to.newline()

def generate_property_id_bit_set(self, *, to, name, iterable, mapping_to_property=lambda p: p):
to.write(f"const WTF::BitSet<numCSSProperties> {name} = ([]() -> WTF::BitSet<numCSSProperties> {{")

with to.indent():
to.write(f"WTF::BitSet<numCSSProperties> result;")

for item in iterable:
to.write(f"result.set({mapping_to_property(item).id});")

to.write(f"return result;")
to.write(f"}})();")
to.newline()

# Generates `CSSPropertyNames.h` and `CSSPropertyNames.cpp`.
class GenerateCSSPropertyNames:
Expand Down Expand Up @@ -2839,12 +2851,25 @@ def generate_css_property_names_gperf(self):
default="return { };"
)

self.generation_context.generate_property_id_switch_function_bool(
self.generation_context.generate_property_id_bit_set(
to=writer,
signature="bool CSSProperty::isColorProperty(CSSPropertyID id)",
name="CSSProperty::colorProperties",
iterable=(p for p in self.properties_and_descriptors.style_properties.all if p.codegen_properties.color_property)
)

physical_properties = []
for _, property_group in sorted(self.generation_context.properties_and_descriptors.style_properties.logical_property_groups.items(), key=lambda x: x[0]):
kind = property_group["kind"]
destinations = LogicalPropertyGroup.logical_property_group_resolvers["physical"][kind]
for property in [property_group["physical"][a_destination] for a_destination in destinations]:
physical_properties.append(property)

self.generation_context.generate_property_id_bit_set(
to=writer,
name="CSSProperty::physicalProperties",
iterable=sorted(list(set(physical_properties)), key=lambda x: x.id)
)

self.generation_context.generate_property_id_switch_function(
to=writer,
signature="UChar CSSProperty::listValuedPropertySeparator(CSSPropertyID id)",
Expand Down
14 changes: 14 additions & 0 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4539,6 +4539,20 @@ bool Element::hasRunningTransitions(PseudoId pseudoId) const
return false;
}

const AnimatablePropertyToTransitionMap* Element::completedTransitionsByProperty(PseudoId pseudoId) const
{
if (auto* animationData = animationRareData(pseudoId))
return &animationData->completedTransitionsByProperty();
return nullptr;
}

const AnimatablePropertyToTransitionMap* Element::runningTransitionsByProperty(PseudoId pseudoId) const
{
if (auto* animationData = animationRareData(pseudoId))
return &animationData->runningTransitionsByProperty();
return nullptr;
}

AnimationCollection& Element::ensureAnimations(PseudoId pseudoId)
{
return ensureAnimationRareData(pseudoId).animations();
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/dom/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,9 @@ class Element : public ContainerNode {
bool hasRunningTransitions(PseudoId) const;
AnimationCollection& ensureAnimations(PseudoId);

const AnimatablePropertyToTransitionMap* completedTransitionsByProperty(PseudoId) const;
const AnimatablePropertyToTransitionMap* runningTransitionsByProperty(PseudoId) const;

AnimatablePropertyToTransitionMap& ensureCompletedTransitionsByProperty(PseudoId);
AnimatablePropertyToTransitionMap& ensureRunningTransitionsByProperty(PseudoId);
CSSAnimationCollection& animationsCreatedByMarkup(PseudoId);
Expand Down
Loading

0 comments on commit 66eb8a0

Please sign in to comment.