Optimize string comparisons to avoid substring allocations - #292
Merged
Merged
Conversation
Profiled with --prof on Tailwind's dist CSS (3.5MB, ~560k nodes) to find real hot spots rather than guessing: - ValueNodeParser.is_whitespace_inline() scanned every character of a token to check if it was all-whitespace. The tokenizer already emits whitespace runs as a single TOKEN_WHITESPACE token (comments never end up inside one - verified the tokenizer splits a whitespace run around a comment into two separate TOKEN_WHITESPACE tokens), so a direct token_type check is behaviorally identical and O(1) instead of O(n). This runs on every token during value/function/media-feature parsing. - DIMENSION nodes' .value and .unit getters each called parse_dimension(), which allocates an object plus two substrings, to get half of what they needed. Split out dimension_number_end() (the split-point scan, no allocation) so each getter slices only its own part. - parse_function_node() (every var()/calc()/rgb()/url()/... in a value) and parse-selector.ts's parenthesized-pseudo-class handling (:not(), :is(), :nth-child(), ...) each allocated a substring just to compare it against a handful of literal function/pseudo names. Added str_equals_range() for allocation-free offset-based comparison and used it in both places. Added a regression test for comments between value tokens and function arguments, the scenario the whitespace-check change depends on. All 1428 existing + new tests pass; no API changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
| 📦 Package | 📏 Base Size | 📏 Source Size | 📈 Size Change |
|---|---|---|---|
| @projectwallace/css-parser | 45.5 kB | 45.9 kB | +468 B |
commit: |
is_whitespace_inline() was reduced to a single field comparison last commit, so the wrapping method call was pure overhead on a path that runs for every token during value parsing. Inlined all 9 call sites as direct token_type === TOKEN_WHITESPACE checks, reusing the local token_type/tt/t variable already captured in scope where one exists instead of re-reading the field through this.lexer. Also fixes a real TS narrowing conflict this surfaced: two spots read this.lexer.token_type directly right after assigning it to a local (t) and checking that local, which left TS unable to prove the raw field read could still be TOKEN_WHITESPACE. Using the local everywhere sidesteps it and matches the surrounding code's existing style. All 1428 tests pass; no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
Author
|
after: |
Member
Author
|
before: |
Member
Author
|
Delta on your before/after numbers:
Parse-only gains land in the 2-4% range on the three smaller/mid files, roughly flat on Tailwind (its size dwarfs the per-token savings from this PR, and single-run noise on a 3.5MB parse is bigger than the effect). Walk-only shows a consistent ~1% dip across all four files, but Matches what I estimated from CPU-time profiling before opening this PR (~3-12% on parse-heavy real-world CSS, closer to flat on Tailwind). Generated by Claude Code |
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
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
This PR optimizes string comparison operations throughout the codebase by introducing a new
str_equals_range()function that performs case-insensitive equality checks directly on source ranges without allocating substrings first. This reduces memory pressure and improves performance in hot code paths.Key Changes
New
str_equals_range()function instring-utils.ts: Performs case-insensitive equality comparison between a source string range[start, end)and a literal string without allocating a substring. Uses bitwise OR to normalize ASCII uppercase to lowercase in a single operation.Selector parser optimization: Refactored
parse_pseudo_class_function()andis_nth_pseudo()to use offset-based comparisons instead of substring allocation. This avoids creating temporary strings for every parenthesized pseudo-class (:not(),:is(),:nth-child(), etc.).Value node parser optimization:
is_whitespace_inline()implementation with a direct token type check (TOKEN_WHITESPACE) instead of scanning characters, reducing O(n) to O(1).str_equals_range()forif(),url(), andsrc()functions.TOKEN_WHITESPACEimport for the optimized whitespace check.Dimension parsing refactor: Extracted
dimension_number_end()helper function to find where a dimension's numeric part ends, allowing callers to slice only the part they need without allocating both value and unit substrings when only one is needed.CSS node optimization: Updated
valueandunitgetters to usedimension_number_end()for more efficient parsing without unnecessary allocations.Test coverage: Added tests for comment handling between value tokens and function arguments to ensure whitespace/comment skipping works correctly.
Implementation Details
str_equals_range()function uses a length check first for early exit, then compares characters withch |= 32to normalize ASCII uppercase (A-Z: 65-90) to lowercase (a-z: 97-122) without branching.str_equals_range()must be lowercase by contract.https://claude.ai/code/session_01FXs4NU4Mm1TjRXsmUw9ufe