[CSS] Early return from matching when doing invalidation #39737

mdubet · 2025-01-30T01:31:13Z

`ce251e4`

[CSS] Early return from matching when doing invalidation
https://bugs.webkit.org/show_bug.cgi?id=286721
rdar://138419832

Reviewed by Antti Koivisto.

For invalidation, we generally don't want to collect rules
but just to know if any rule matches. We can optimize for
this use case by early returning on first match.

 * Source/WebCore/style/ElementRuleCollector.cpp:
 (WebCore::Style::ElementRuleCollector::isFirstMatchModeAndHasMatchedAnyRules const):
 (WebCore::Style::ElementRuleCollector::collectMatchingRules):
 (WebCore::Style::ElementRuleCollector::transferMatchedRules):
 (WebCore::Style::ElementRuleCollector::collectMatchingRulesForList):
 (WebCore::Style::ElementRuleCollector::matchAllRules):
 (WebCore::Style::ElementRuleCollector::hasAnyMatchingRules): Deleted.

 Renamed to matchesAnyRules().

 * Source/WebCore/style/ElementRuleCollector.h:
 * Source/WebCore/style/StyleSharingResolver.cpp:
 (WebCore::Style::SharingResolver::styleSharingCandidateMatchesRuleSet const):

Canonical link: https://commits.webkit.org/289611@main

fc45b11

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-01-30T01:31:20Z

EWS run on previous version of this PR (hash bd604f8)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

nt1m

I'm not sure whether this is the good approach or not, but here's a style nit

Source/WebCore/style/ElementRuleCollector.cpp

webkit-early-warning-system · 2025-01-30T16:53:02Z

EWS run on previous version of this PR (hash afab8f2)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 win~~
✅ 🧪 bindings	~~🛠 ios-sim~~	~~🛠 mac-AS-debug~~	~~🧪 wpe-wk2~~	~~🧪 win-tests~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	~~🛠 wpe-cairo~~
	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🛠 gtk~~
	~~🛠 vision~~	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	~~🛠 vision-sim~~	~~🧪 mac-wk2-stress~~	~~🧪 api-gtk~~
	⏳ 🧪 vision-wk2	~~🧪 mac-intel-wk2~~	~~🛠 playstation~~
	~~🛠 tv~~	~~🛠 mac-safer-cpp~~
	~~🛠 tv-sim~~
	~~🛠 watch~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2025-01-30T16:57:22Z

EWS run on previous version of this PR (hash 00508c0)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	⏳ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Source/WebCore/style/ElementRuleCollector.cpp

Source/WebCore/style/ElementRuleCollector.h

anttijk · 2025-01-30T18:36:49Z

Source/WebCore/style/ElementRuleCollector.cpp

We still do some unnecessary work like hash lookups in ElementRuleCollector::collectMatchingRules after the first match. I think this ok for now though, it can be improved later.

webkit-early-warning-system · 2025-01-30T20:30:06Z

EWS run on previous version of this PR (hash 5cc3611)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-ews-buildbot · 2025-01-31T06:34:08Z

Failed mac-AS-debug-wk2 checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #39737 from merge queue.

webkit-ews-buildbot · 2025-01-31T06:34:23Z

Safe-Merge-Queue: Build #47392.

webkit-ews-buildbot · 2025-01-31T16:18:55Z

Failed mac-AS-debug-wk2 checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #39737 from merge queue.

webkit-ews-buildbot · 2025-01-31T16:19:10Z

Safe-Merge-Queue: Build #47431.

webkit-early-warning-system · 2025-01-31T16:41:44Z

EWS run on current version of this PR (hash fc45b11)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

mdubet · 2025-01-31T16:42:08Z

mac-AS-debug failures are unrelated

https://bugs.webkit.org/show_bug.cgi?id=286721 rdar://138419832 Reviewed by Antti Koivisto. For invalidation, we generally don't want to collect rules but just to know if any rule matches. We can optimize for this use case by early returning on first match. * Source/WebCore/style/ElementRuleCollector.cpp: (WebCore::Style::ElementRuleCollector::isFirstMatchModeAndHasMatchedAnyRules const): (WebCore::Style::ElementRuleCollector::collectMatchingRules): (WebCore::Style::ElementRuleCollector::transferMatchedRules): (WebCore::Style::ElementRuleCollector::collectMatchingRulesForList): (WebCore::Style::ElementRuleCollector::matchAllRules): (WebCore::Style::ElementRuleCollector::hasAnyMatchingRules): Deleted. Renamed to matchesAnyRules(). * Source/WebCore/style/ElementRuleCollector.h: * Source/WebCore/style/StyleSharingResolver.cpp: (WebCore::Style::SharingResolver::styleSharingCandidateMatchesRuleSet const): Canonical link: https://commits.webkit.org/289611@main

webkit-commit-queue · 2025-01-31T18:00:05Z

Committed 289611@main (ce251e4): https://commits.webkit.org/289611@main

Reviewed commits have been landed. Closing PR #39737 and removing active labels.

mdubet self-assigned this Jan 30, 2025

mdubet added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jan 30, 2025

nt1m reviewed Jan 30, 2025

View reviewed changes

Source/WebCore/style/ElementRuleCollector.cpp Outdated Show resolved Hide resolved

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 30, 2025

mdubet removed the merging-blocked Applied to prevent a change from being merged label Jan 30, 2025

mdubet force-pushed the early-return-selector-matching branch from afab8f2 to 00508c0 Compare January 30, 2025 16:57

anttijk approved these changes Jan 30, 2025

View reviewed changes

mdubet changed the title ~~[CSS] Early return if possible when doing style matching~~ [CSS] Early return from matching when doing invalidation Jan 30, 2025

mdubet force-pushed the early-return-selector-matching branch from 00508c0 to 5cc3611 Compare January 30, 2025 20:30

mdubet added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jan 30, 2025

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 31, 2025

webkit-ews-buildbot removed the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jan 31, 2025

mdubet added safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks and removed merging-blocked Applied to prevent a change from being merged labels Jan 31, 2025

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 31, 2025

webkit-ews-buildbot removed the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jan 31, 2025

mdubet removed the merging-blocked Applied to prevent a change from being merged label Jan 31, 2025

mdubet force-pushed the early-return-selector-matching branch from 5cc3611 to fc45b11 Compare January 31, 2025 16:41

mdubet added the merge-queue Applied to send a pull request to merge-queue label Jan 31, 2025

webkit-commit-queue force-pushed the early-return-selector-matching branch from fc45b11 to ce251e4 Compare January 31, 2025 18:00

webkit-commit-queue merged commit ce251e4 into WebKit:main Jan 31, 2025

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 31, 2025

[CSS] Early return from matching when doing invalidation #39737

[CSS] Early return from matching when doing invalidation #39737

Uh oh!

Conversation

mdubet commented Jan 30, 2025 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ce251e4

Uh oh!

webkit-early-warning-system commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nt1m left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

webkit-early-warning-system commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anttijk Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-ews-buildbot commented Jan 31, 2025

Uh oh!

webkit-ews-buildbot commented Jan 31, 2025

Uh oh!

webkit-ews-buildbot commented Jan 31, 2025

Uh oh!

webkit-ews-buildbot commented Jan 31, 2025

Uh oh!

webkit-early-warning-system commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdubet commented Jan 31, 2025

Uh oh!

webkit-commit-queue commented Jan 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mdubet commented Jan 30, 2025 •

edited by webkit-early-warning-system

Loading

`ce251e4`

webkit-early-warning-system commented Jan 30, 2025 •

edited

Loading

nt1m left a comment •

edited

Loading

webkit-early-warning-system commented Jan 30, 2025 •

edited

Loading

webkit-early-warning-system commented Jan 30, 2025 •

edited

Loading

webkit-early-warning-system commented Jan 30, 2025 •

edited

Loading

webkit-early-warning-system commented Jan 31, 2025 •

edited

Loading