perf(core): skip-unchanged CSS re-application, traversal cleanups#11308
Open
NathanWalker wants to merge 3 commits into
Open
perf(core): skip-unchanged CSS re-application, traversal cleanups#11308NathanWalker wants to merge 3 commits into
NathanWalker wants to merge 3 commits into
Conversation
…ching and iOS layout - Observable.notify skips deprecated global-handler machinery (constructor name resolution + 4 hash lookups per event) when no global handlers are registered; listener dispatch uses .call instead of .apply([data]); hasListeners uses a direct read; _indexOfListener drops the closure. - isResetValue/isCssWideKeyword bail out with a single typeof check for non-string values (they run at the top of every property setter). - getSelectorCandidates replaces O(n*buckets) reduce+concat with push-based accumulation; AttributeSelector normalizes ignoreCase value once at construction; media-query string splits are cached; selector match paths use indexed loops instead of every/forEach closures and drop string-keyed method dispatch. - addPseudoClass/deletePseudoClass no longer allocate an aliased-states array per visual-state transition; classNameProperty.valueChanged drops per-change closures; fix latent getViewByDomId bug comparing the root's _domId instead of each child's. - iOS toDevicePixels/toDeviceIndependentPixels/getDisplayDensity cache the screen scale instead of resolving window->screen->scale through the ObjC bridge on every conversion (mirrors the Android density cache). - Add vitest micro-benchmarks (packages/core/__tests__/benchmarks) and unit tests locking dispatch order, thisArg binding, self-removal during dispatch, global-handler semantics, ignoreCase attribute matching and query result ordering. Measured: automated iOS suite 1808 OK / 0 failed before and after; duration 49,891ms -> 45,469ms / 43,019ms (two runs). Micro-benchmarks: selector query +19-39%, Observable.notify +22-34%, attribute match +19%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…re pass The config referenced @typescript-eslint/no-extra-semi, which was removed in typescript-eslint v8, causing a rule-not-found error on every linted file. Removing it surfaced pre-existing issues, addressed as follows: - Disable the v8 successors of the already-disabled ban-types rule (no-unsafe-function-type, no-wrapper-object-types, no-empty-object-type) plus no-unsafe-declaration-merging (interface+class merging is a core NativeScript pattern), and allow short-circuit/ternary expression statements, matching existing code style. - Stop linting generated iOS typings (platforms/) and the vendored css parser (css/lib/) in packages/core. - Drop parserOptions.project from packages/core lint config: no type-aware rules are in use, and it caused parsing errors for ~20 handwritten d.ts files not included in any tsconfig. - Code fixes: do..while(true) -> while(true) in CSS3Parser, prefer-const in inspector_modules, module -> namespace in file-system/trace d.ts files, eslint-disable for the required triple-slash reference in index.d.ts. nx lint core, nx test core (192 passed) and nx build core all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, traversal cleanups - CssState.setPropertyValues: fix the pre-existing ordering bug where delete oldProperties[property] ran before the "skip unchanged" check, making it dead code — unchanged declarations were re-applied through the property system on every dynamic-state change (every :active toggle). Scoped css-variables are still always re-registered because resetScopedCssVariables clears them earlier in the same pass. - eachLayoutChild allocates 2 closures per measure/layout pass instead of N+1; onLoaded/onUnloaded traversals use shared module-level callbacks (dispatching via child.parent to preserve loadView/unloadView overrides, e.g. TabView). - query() appends media-query candidates via indexed loop instead of push-spread (avoids arg-spread limits on large candidate sets). - Benchmarks: theme-scale stylesheet + recycled-rows query bench; new spec tests for repeated-query consistency incl. media-query non-accumulation. Note: selector candidate memoization keyed by (cssType, id, classes) was prototyped and measured to be a net LOSS at realistic scale (cached lookup 273k ops/s vs 294k direct; recycled rows -21%) — string key building plus Map hashing costs more than the push-based rebuild from phase 1, which already made candidate sets cheap. Rejected; documented here so it is not re-attempted without new evidence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
View your CI Pipeline Execution ↗ for commit 37d2b8e
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
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.
Considerations
Additional notes to be considered aside (above/beyond) phase 1 perf opts here: #11307
Note: selector candidate memoization keyed by (cssType, id, classes) was prototyped and measured to be a net LOSS at realistic scale (cached lookup 273k ops/s vs 294k direct; recycled rows -21%); string key building plus Map hashing costs more than the push-based rebuild from phase 1, which already made candidate sets cheap.
query— button w/ id + 4 classesquery— 20 recycled list rows (4 classes each)Building the string cache key (per-class concatenation) plus hashing it for
Map.getcosts more than the push-based candidate rebuild that phase 1 already made cheap; per-node candidate sets are bounded (universal + id + type + k class buckets, realistically < 100 entries), andaccumulateChanges+sortdominate query cost regardless. Mentioning here so it isn't re-attempted without new evidence.