Skip to content

Resolve hot-path: stop copying every value, fix whole-collection exclusion - #4

Merged
ndreno merged 2 commits into
mainfrom
perf/resolve-copy-and-exclusion-prefix
Sep 11, 2026
Merged

ndreno merged 2 commits into
mainfrom
perf/resolve-copy-and-exclusion-prefix

Conversation

@ndreno

@ndreno ndreno commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

The two smaller resolve-path follow-ups from the engine review. Independent of each other; grouped because both touch resolution.

1. Stop copying every inspected value (perf)

evaluate_step did value.value.to_vec() for every resolved value before testing it, even for a rule with no transformations, undoing the borrowing the resolver does to avoid exactly that copy. A rule set resolves thousands of values per request and most operators read the value as-is.

Candidates are now Cow<[u8]>:

  • No transformations: the value is tested in place, zero copies.
  • A no-op transformation (t:none, or t:lowercase on already-lowercase input) returns a borrow instead of allocating.
  • Only the value that actually matches is owned, once, for MATCHED_VAR.
  • multiMatch still materialises the value before and after each transformation, since it tests each stage.

2. Whole-collection exclusion matched by string prefix (correctness)

A selector-less exclusion (!ARGS) removed every resolved value whose name began with the collection name — so it also stripped ARGS_GET:x and ARGS_NAMES:x, since those start with "ARGS". A rule excluding one collection while inspecting a longer-named sibling silently lost members.

The exclusion now resolves each value name into (collection, member) and matches the collection exactly: a scalar PREFIX, or a map member PREFIX:member. A name that merely starts with the prefix no longer matches. Selector-bearing exclusions were already anchored by the : split, so CRS (which always writes a selector) is unaffected.

Verification

  • 172 unit tests pass (+3: no-op-transform match, no-transform match, and the ARGS_GET|!ARGS regression guard); clippy and fmt clean.
  • Real CRS 4.9.0 still compiles to 590 rules with the same 4 refusals, and cookie exclusions still behave (__utmz allowed, session blocked).
  • The transformation-order and multiMatch tests pass unchanged, so Refuse invalid selectors and backward skipAfter at compile time #1 is behaviour-preserving.

evaluate_step copied every resolved value with to_vec before testing it, even
when the rule had no transformations, so the borrowing the resolver does to
avoid copying was undone one line later. A rule set resolves thousands of
values per request, and most operators read the value as-is.

Candidates are now Cow: without transformations the value is tested in place
with no copy, and a transformation that changes nothing (t:none, or t:lowercase
on already-lowercase input) returns a borrow rather than allocating. Only the
value that actually matches is owned, once, for MATCHED_VAR. multiMatch still
materialises the value before and after each transformation, since it tests
each stage.

Behaviour is unchanged: the transformation-order and multiMatch tests pass as
before, and the Core Rule Set still compiles to 590 rules and blocks the same
traffic.
A selector-less exclusion (`!ARGS`) removed every resolved value whose name
began with the collection name, so it also stripped `ARGS_GET:x` and
`ARGS_NAMES:x`, whose names start with "ARGS". A rule that excluded one
collection while inspecting a longer-named sibling silently lost members.

The exclusion now resolves the value's name into (collection, member) and keeps
it only when the collection matches exactly: a scalar named `PREFIX`, or a map
member named `PREFIX:member`. A name that merely starts with the prefix, like
ARGS_GET against ARGS, no longer matches. Selector-bearing exclusions were
already anchored by the `:` split and are unaffected; the Core Rule Set, which
always writes a selector, is unchanged.
@ndreno
ndreno merged commit 472c6e8 into main Sep 11, 2026
8 checks passed
@ndreno
ndreno deleted the perf/resolve-copy-and-exclusion-prefix branch September 11, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant