Bug 190106
Summary: | Style::determineChange() does a full RenderStyle compare when it needs != | ||
---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> |
Component: | CSS | Assignee: | Nobody <webkit-unassigned> |
Status: | NEW | ||
Severity: | Normal | CC: | ahmad.saleem792, koivisto, simon.fraser, zalan |
Priority: | P2 | ||
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified |
Simon Fraser (smfr)
Style::determineChange() has:
if (s1 != s2) {
if (s1.inheritedNotEqual(&s2))
return Inherit;
if (s1.alignItems() != s2.alignItems() || s1.justifyItems() != s2.justifyItems())
return Inherit;
return NoInherit;
}
That s1 != s2 is a deep, full compare since:
bool operator!=(const RenderStyle& other) const { return !(*this == other); }
We should probably implement operator != for the common types, which early returns.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Antti Koivisto
> We should probably implement operator != for the common types, which early
> returns.
In case styles are equal operator!= is not usually very deep. It just compares a small set of pointers and bits for equality.
Simon Fraser (smfr)
(In reply to Antti Koivisto from comment #1)
> > We should probably implement operator != for the common types, which early
> > returns.
>
> In case styles are equal operator!= is not usually very deep. It just
> compares a small set of pointers and bits for equality.
Right, but it's the != case where only one or two properties changed that affect benchmark and animated content performance (e.g. things that just change transform).
Ahmad Saleem
We have following:
https://github.com/WebKit/WebKit/blob/6344a735d399205f67cc29e54ac57e23a803ea4f/Source/WebCore/style/StyleChange.cpp#L34
and it seems to have added some cases "PseudoId::FirstLetter", "columnSpan" etc.
Do we need to tackle this anymore or Webkit now able to do early returns similar to proposed in Comment 0?
Appreciate if someone can comment based on current state and we can mark this bug accordingly. Thanks!