Conversation
Style.of() merged the parent's already-flattened custom-property (variable) map with the current element's own declared variables, then re-flattened the whole merged set unconditionally - even when the element declared no variables of its own, in which case the merge is a no-op and re-flattening reproduces the parent's map exactly. Profiled sia-r65 (~140s on a 10k-node real-world page) with Node's CPU profiler: 39.5% of all execution time was inside Hash.writeString, traced through Map.set -> Variable.flatten -> Style.of -> Style.from. The page defines 1,742 distinct custom properties (a design-token system); every style computation re-substituted var() references across however many of those were in scope, from scratch, regardless of whether anything could have changed. This isn't rule- or context-specific - it taxes every rule on any page with a nontrivial custom-property set. sia-r65 just makes it visible because its per-target scan produces far more uncached style computations than most rules ever do. Verified identical pass/fail/cantTell/inapplicable outcomes across every rule in the rule set, on both a small and a 10k-node fixture, with and without the fix. On the 10k-node fixture, a single fresh evaluation (not a repeat-evaluation benchmark artifact, unlike the Context-interning fix): sia-r65 129.5s -> 39.6s (-69.4%), sia-r62 218ms -> 133ms (-39.0%).
🦋 Changeset detectedLatest commit: 1e63589 The changes in this PR will be included in the next version bump. This PR includes changesets to release 76 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Style.of()merges the parent's already-flattened custom-property (variable) map with the current element's own declared variables, then re-flattens the whole merged set — unconditionally, even when the element declares no variables of its own (the common case), in which case the merge is a no-op and re-flattening just reproduces the parent's map at full cost.Profiled
sia-r65(~140s on a 10k-node real-world page) with Node's CPU profiler: 39.5% of all execution time was insideHash.writeString, traced throughMap.set → Variable.flatten → Style.of → Style.from. The fixture defines 1,742 distinct custom properties (a design-token system); every style computation was re-substitutingvar()references across however many of those were in scope, from scratch, regardless of whether anything could have changed.This isn't rule- or context-specific — it taxes every rule on any page with a nontrivial custom-property set.
sia-r65just makes it visible because its per-target scan produces far more uncached style computations than most rules ever do.Fix
Skip
Variable.flatten()and reuse the parent'svariablesmap directly when the current element's ownVariable.gather()result is empty — safe becauseparent.variablesis already flattened by construction (everyStyleinstance is built through this same function), so re-flattening it with nothing new merged in is a no-op.Measured impact
Verified identical pass/fail/cantTell/inapplicable outcomes across every rule in the rule set, on both a small and a 10k-node fixture, with and without the fix — this is a caching fix, not a behavior change.
On the 10k-node fixture, a single fresh evaluation (not a repeat-evaluation benchmark artifact — confirmed via isolated single-pass timing, unlike the earlier
Context-interning PR #2168):Full rule-set run (all ~90 rules) on the same fixture: total duration dropped from ~143s to ~44s.
Test plan
yarn build packages/alfa-style(+ downstream:alfa-rules,alfa-act,alfa-cascade,alfa-aria)yarn test packages/alfa-style packages/alfa-rules packages/alfa-act packages/alfa-cascade packages/alfa-aria— 2302/2302 passingyarn changeset status— clean🤖 Generated with Claude Code