Skip to content

perf(view): reduce linkTo/URLFor overhead on JVM engines - #4192

Merged
bpamiri merged 10 commits into
developfrom
peter/linkto-urlfor-cost-4151
Oct 5, 2026
Merged

bpamiri merged 10 commits into
developfrom
peter/linkto-urlfor-cost-4151

Conversation

@bpamiri

@bpamiri bpamiri commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

What

Three byte-identical micro-optimizations to the linkTo() / URLFor() / $tag() path, which the .rcx report flagged as a large share of a list page's render cost. Each is a separate commit with its own before/after.

Refs #4151 (does not close it — see "RustCFML" below).

The three wins

  1. De-dup $args (W1) — linkTo already normalises its arguments, then URLFor re-ran a second generic $args pass. URLFor gains a declared $argsResolved param; when linkTo sets it, URLFor skips the generic pass but still applies its own set(functionName="URLFor", …) defaults. Declaring the flag as a real argument keeps the sentinel out of the query string.
  2. Literal placeholder substitution (W2) — $urlForSubstituteVariables ran two regex ops per route variable (ReFind + ReReplace on [prop]/[*prop]). Route tokens have no regex metacharacters, so it now uses literal Find/Replace (first-occurrence, matching ReReplace's one scope). A value containing \ or $ stays on the ReReplace path (those are replacement metacharacters whose behaviour is engine-specific).
  3. Resolve the encode flag once (W3) — $tag called $get("encodeHtmlAttributes") per attribute via $tagAttribute; it now resolves it once and passes it in ($tagAttribute falls back to $get for any external caller). The attribute sort/ordering is unchanged.

Before / after — linkTo(controller/action/key), µs/call

engine develop this PR delta
Adobe 2023 375 354 ~5.6%
Lucee 7 56 53 ~5%
RustCFML 78 78 ~0%

$element on Adobe 2023: 256 → 235 (~8%).

RustCFML (why this is "Refs", not "Closes")

These wins remove slow $get and regex-compile overhead — costs that are high on the JVM. RustCFML's cost is in O(index) primitives (Mid/Find/string building/struct copies) that these changes don't touch, so its linkTo is unchanged at 78µs. A follow-up re-profiles RustCFML's 78µs at the primitive level to find #4151's real fix (or feed the RustCFML upstream batch).

Gate

A differential/characterization spec (linkToUrlForDifferentialSpec) pins the output byte-identical: golden strings for the engine-independent corpus (deterministic URLs; $tag sorts attributes), a reference-copy of $urlForSubstituteVariables asserting the live function equals frozen develop per-engine (incl. \1/$1 key values), and URLFor($argsResolved=true) == URLFor(false) incl. an app-level override + a no-$argsResolved-leak check. It passes on develop unchanged and stays green across all three commits.

Full compat matrix + before/after on all 5 engines. (The url.capture branch in the spec is a temporary golden-capture aid; removed before merge.)

🤖 Generated with Claude Code

bpamiri and others added 4 commits October 3, 2026 02:39
… on develop)

Characterization spec that pins the exact output of a linkTo/URLFor corpus so the
planned per-call cost reductions (de-dup $args, literal Replace in
$urlForSubstituteVariables, lighter $tag) stay byte-identical. Two mechanisms:

- End-to-end GOLDEN strings for engine-independent cases (deterministic URLs;
  $tag already sorts attributes alphabetically, so tag output is order-stable).
  These guard the de-dup-$args call-flow change and the $tag change, and assert
  the internal $argsResolved sentinel never leaks into output.
- A verbatim REFERENCE COPY of $urlForSubstituteVariables with a sub-corpus that
  includes regex-backref values (\1, $1) in a pattern variable. That behaviour is
  engine-specific (the pattern has no capture groups), so the live function is
  asserted equal to the frozen reference on the running engine rather than a fixed
  golden. Guards the literal-Replace change.

No production change yet; this is the gate the optimization commits must keep green.
Draft for 4.3 (#4151).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…d them (#4151 W1)

linkTo runs $args(name="linkTo") then calls URLFor, which ran $args(name="URLFor")
again on the same struct — a second generic normalisation pass (the $get(
"showErrorInformation") check and the empty reserved/required/combine loops). On
Adobe that pass measured ~200us.

URLFor gains a declared boolean $argsResolved param (default false). linkTo sets it
true before calling URLFor. When set, URLFor skips the generic $args pass but STILL
applies its own app-level defaults via structAppendDefaults(functions.URLFor), so
set(functionName="URLFor", …) overrides keep reaching linkTo URLs. Declaring
$argsResolved as a real argument keeps the sentinel out of the generated query string.

Byte-identical: the differential gate now also asserts URLFor($argsResolved=true)
equals URLFor($argsResolved=false) (incl. with an app-level override) and that the
sentinel never appears in output. before/after (Lucee 7): linkTo 56 -> 54 us/call;
the larger win is on Adobe, where the duplicated $args pass is far costlier.

Draft for 4.3 (#4151).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…d of regex (#4151 W2)

$urlForSubstituteVariables ran two regex operations per route variable — ReFind to
test the `[prop]` / `[*prop]` placeholder presence and ReReplace to substitute the
value. Route variable names are alphanumeric tokens with no regex metacharacters, so
literal Find/Replace of the exact placeholder is equivalent and cheaper (the regex is
the per-call hot spot on RustCFML and Adobe especially).

Presence is now Find("[prop]") / Find("[*prop]"); substitution replaces the first
matching literal placeholder with first-occurrence Replace (matching ReReplace's one
scope). A value containing a backslash or `$` is kept on the ReReplace path, because
those are regex-replacement metacharacters whose behaviour (no capture groups here) is
engine-specific — the frozen reference-copy in the differential spec pins that per
engine, and this fallback preserves it exactly.

Byte-identical: the differential gate's $urlForSubstituteVariables reference-copy test
(incl. \1 / $1 key values) stays green. before/after (Lucee 7): URLFor 26 -> 24,
linkTo 54 -> 52 us/call; larger on RustCFML/Adobe where the per-variable regex is
costlier.

Draft for 4.3 (#4151).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…per attribute (#4151 W3)

$tag called $tagAttribute per attribute, and each $tagAttribute called
$get("encodeHtmlAttributes"). $tag now resolves that flag once and passes it in;
$tagAttribute gains an optional encodeHtmlAttributes argument and only falls back to
$get when a caller omits it (not a signature default, so the $get is never evaluated
at parse time). The attribute sort/ordering is unchanged.

Byte-identical: the differential gate's $element goldens (5-attribute and
skipStartingWith cases) stay green. before/after (Adobe 2023, where $get is costliest):
$element 256 -> 235 us/call, linkTo 375 -> 354; flat on Lucee (its $get is ~free).

Draft for 4.3 (#4151).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ 6052f79 (draft, per commit): CHANGES (one spec addition), otherwise approve

I reviewed this commit by commit against the #4151 design notes.

  • d073fac (gate). It's a real oracle: it passes on develop, with goldens cached and uncached, a verbatim reference copy of $urlForSubstituteVariables including the \1 and $1 values, a 5-attribute $element golden (sorted order pinned) and a skipStartingWith case.
  • e017d5d (W1). It's correct. With $argsResolved true, URLFor skips only the generic pass and still runs structAppendDefaults(…, functions.URLFor). That's exactly what the old $args did for URLFor (no reserved, combine or required at the call site), so set(functionName="URLFor", …) still reaches linkTo URLs. The probe(true) == probe(false) test, with an app-level override mutated in, proves it. Declaring the sentinel as a real argument keeps it out of the params, and the no-leak assertion holds on both paths.
  • 6580b46 (W2). The logic is right:
    • presence is Find("[prop]") || Find("[*prop]"), which is case-sensitive like ReFind;
    • substitution replaces whichever literal placeholder appears first, mirroring ReReplace's single first match of \[\*?prop\];
    • any value containing \ or $ stays on ReReplace, which also covers Lucee's \U, \L and \E replacement escapes.
  • 6052f79 (W3). It hoists $get("encodeHtmlAttributes") once per tag. The ListSort(…, "textnocase") ordering is untouched, and external $tagAttribute callers keep the per-call fallback (an "" default, then IsBoolean, then $get), so the parse-time default concern is avoided.

Requested before un-drafting. W2's new branch, which picks [*prop] over [prop] by position, has no reference-copy probe. All five probes use plain [prop] placeholders. Please add at least:

  • a wildcard route, rv = "/files/[*path]" (with a /-containing value, encode on and off);
  • a pattern containing both [*key] and [key] in each order, so the "earliest wins" logic is checked against ReReplace on every engine.

That's the only new control flow in the PR that the gate doesn't exercise.

The numbers are honest: a JVM-only win, about 6% on Adobe linkTo, with RustCFML flat and its O(index) hot spot deferred. Fine for a 4.3 draft. Matrix 37118200529 was still running when I reviewed.

 gate

The reference-copy probes for $urlForSubstituteVariables were all plain [prop], so
W2's literal-Replace path for wildcard [*prop] placeholders and its "replace the
earliest of [prop]/[*prop]" branch had no oracle (rev1-r3). Add probes: a wildcard
[*path] with a slash value (encode on and off), both [key] and [*key] in each order,
and a wildcard value carrying a backslash (the ReReplace fallback). Each asserts the
live $urlForSubstituteVariables equals the frozen develop reference on the running
engine. Gate stays green on the W1+W2+W3 branch.

Draft for 4.3 (#4151).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 re-review @ 9b23f8d: APPROVE (merge-ready once matrix 37118431060 is green and it leaves draft)

The delta since 6052f795f is spec-only: no change under global/ or view/. The reference-copy gate now has an oracle for W2's new branch:

  • [*path] with a slash value, encode off and on;
  • both placeholders in each order (/a/[*key]/b/[key] and /a/[key]/b/[*key]), which directly checks the "earliest placeholder wins" choice against ReReplace on every engine;
  • wildcard values with \1 and $1, which exercise the fallback.

That closes my only gap. W1, W2 and W3 are as reviewed at 6052f795f.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Wheels Test Results

216 871 tests   212 747 ✅  1h 15m 53s ⏱️
 16 608 suites    4 124 💤
     32 files          0 ❌

Results for commit c3f688a.

♻️ This comment has been updated with latest results.

…outes

The full compat suite runs route-manipulating specs before this one, leaving the route
table (and the application-scoped urlForCache) in a state the corpus did not expect —
the matrix failed with "Could not find the `posts`/`editPost` route". Isolated runs
passed because the default app routes were intact.

Install a known route table in beforeEach ($clearRoutes + a mapKey wildcard and root,
then $setNamedRoutePositions) and restore the real routes in afterEach, resetting the
urlForCache around each test (mirrors linksSpec.cfc). Drop the url_route_root case: the
named-route lookup is sensitive to route-cache state a redefined table doesn't fully
rebuild here, and its pattern substitution is already covered by the
$urlForSubstituteVariables reference-copy test.

Verified: isolated 3/0/0, and the whole view suite 608/0/0 (so it is stable after
linksSpec clears/redefines routes).

Draft for 4.3 (#4151).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 re-review @ 30e712f: CHANGES (the restore is incomplete)

Installing a known route table is the right fix, but the save and restore only cover application.wheels.routes. Two more pieces of route-derived state are global and get changed here:

That's exactly the "state left behind for later specs" class this commit is fixing, just moved downstream.

Fix: mirror linksSpec.cfc (L26-30 and L316-320), the house pattern.

  • Save routes, staticRoutes and namedRoutePositions in beforeEach.
  • Make $clearRoutes() reset all three (routes = [], staticRoutes = {}, namedRoutePositions = {}).
  • Restore all three in afterEach.

#4183 compatibility (orch1's question). #4183 (open: index dynamic routes by method and first two path segments) adds another route-derived structure. When both are on develop, this spec must also save and restore #4183's index, or rely on its staleness guard to rebuild it when routes changes. Whichever merges second should check that. The simplest future-proof shape is a shared test helper that snapshots and restores every route-derived key together, so new indexes don't have to be remembered in each spec.

Dropping url_route_root is fine; the reference copy covers that substitution.

The differential spec's afterEach restored only application.wheels.routes,
but beforeEach also mutates two sibling caches: $setNamedRoutePositions()
*appends* into namedRoutePositions and mapper().end() rebuilds staticRoutes.
Restoring only routes left those two dirty, so a later spec in the full
suite inherited stale indices -- the same stale-index class this gate exists
to catch.

Snapshot and restore all three (routes + staticRoutes + namedRoutePositions)
and clear all three in $clearRoutes(), mirroring linksSpec. Clearing
namedRoutePositions before the append also makes URLFor(route="root")
deterministic again, so url_route_root is restored to the golden corpus.
Remove the url.capture debug branch.

Verified on Lucee 7: view suite 608/0/0; full core suite 6516 pass with
only pre-existing TestClient loopback-port artifacts (unrelated).

Refs #4151

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed @rev1-r3's CHANGES @ 30e712f in 4bf5e64.

The catch (correct): the differential gate's afterEach restored only application.wheels.routes, but beforeEach mutates two sibling caches too — $setNamedRoutePositions() appends into namedRoutePositions and mapper().end() rebuilds staticRoutes. Restoring only routes left those dirty, so a later spec inherited stale indices: the exact stale-index class this gate exists to catch.

Fix (mirrors linksSpec):

  • beforeEach now snapshots all three (routes + staticRoutes + namedRoutePositions) with StructKeyExists-guarded StructCopy/Duplicate.
  • afterEach restores all three.
  • $clearRoutes() now clears all three.
  • Clearing namedRoutePositions before the append also makes URLFor(route="root") deterministic again, so url_route_root is restored to the golden corpus (golden /) with a corrected comment.
  • Removed the url.capture debug branch.

Verified on Lucee 7:

  • view suite 608/0/0 (includes this spec + the route-heavy linksSpec, which sorts after it — the downstream leak test).
  • full core suite 6516 pass, 0 route/view/dispatch failures. The only reds were internal.TestClient* loopback specs ("Expected [200] Actual [0]") — a port-mapping artifact of my hand-rolled one-off container (external 62111 ≠ internal 60007; compose uses 60007:60007), confirmed by docker exec loopback probe. Unrelated to this change.

Fresh compat-matrix dispatched: run 37120749549 on 4bf5e64.

Note per @rev1-r3: #4183 adds another route index — whoever merges second must extend this snapshot/restore to cover it. Filing a follow-up 4.3 issue for a shared $snapshotRoutes()/$restoreRoutes() WheelsTest helper so this list lives in one place.

@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 re-review @ 4bf5e64: APPROVE (merge once matrix 37120749549 is green)

The delta since 30e712f9b is spec-only (no change under global/ or view/).

  • Save. beforeEach snapshots all three route-state structures (routes via Duplicate, staticRoutes and namedRoutePositions via StructCopy).
  • Clear. $clearRoutes() now resets all three before rebuilding, matching linksSpec.cfc. Because $clearRoutes() assigns new structs, the mapper and $setNamedRoutePositions() write into those and never mutate the originals, so the shallow StructCopy snapshots are sufficient.
  • Restore. afterEach puts all three back and resets urlForCache.
  • url_route_root back in the goldens. Clearing namedRoutePositions before the append is also what makes URLFor(route="root") deterministic, so restoring that case is correct.
  • Downstream check. Running the whole view directory (608/0/0) includes linksSpec afterwards, which is a reasonable check that nothing leaks.

Standing note for orch1: when #4183 lands, whichever of #4183 and #4192 merges second should add #4183's route index to this save, clear and restore, unless #4183's staleness guard rebuilds it from routes.

…dobe 2023)

A quoted named argument in a direct call -- `c.URLFor("$argsResolved" = x)` --
is a compile error on Adobe CF 2023 (`MissingNameException`: "each parameter
must have a name"), which zeroed the entire adobe2023 matrix leg across every
database. Adobe misattributes it to the enclosing describe() line, and Lucee
6/7, BoxLang and Adobe 2025 all accept the quoted name, so Lucee-only local
verification missed it (invariant #16 family).

Build the args in a struct and pass them through argumentCollection, which
sidesteps named-argument-name parsing entirely and is byte-identical in effect.

Verified locally, view suite 608/0/0 on all three: adobe2023, adobe2025, lucee7
(Adobe containers restarted after the edit to clear cached classes).

Refs #4151

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

adobe2023 compile fix — d857ffe

The compat-matrix on 4bf5e6408 failed adobe2023 only (all 6 DBs; lucee6/lucee7/adobe2025/boxlang green). Root cause: a quoted named argument in a direct call in the W1 probe —

c.URLFor(controller = "posts", ..., "$argsResolved" = resolved)

— is a compile error on Adobe CF 2023 (MissingNameException: "each parameter must have a name"), which zeroes the entire engine leg. Adobe misattributes it to the enclosing describe() line. Lucee 6/7, BoxLang and Adobe 2025 all accept the quoted name, so my Lucee-only local verification missed it (invariant #16 family). It was pre-existing — adobe2023 was red on 9b23f8db7 and 30e712f9b too, so no prior matrix leg was ever truly green.

Fix: build the args in a struct and pass via argumentCollection (byte-identical in effect, sidesteps named-arg-name parsing):

var probe = (resolved) => {
    var pArgs = {controller = "posts", action = "index", params = "q=a b&x=y", encode = true};
    pArgs["$argsResolved"] = resolved;
    return c.URLFor(argumentCollection = pArgs);
};

Verified locally (Adobe containers restarted post-edit to clear cached classes), view suite 608/0/0 on adobe2023, adobe2025, and lucee7.

Fresh compat-matrix: run 37122451378 on d857ffe.

@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 delta @ d857ffe: code OK. The verdict follows the per-leg read of matrix 37122451378 (in progress), with all six adobe2023 legs checked explicitly.

The fix replaces the quoted named argument "$argsResolved" = resolved in a direct call (Adobe 2023's MissingNameException, the invariant-16 family) with argumentCollection = pArgs, setting the key through bracket notation. The URLFor call stays equivalent. I scanned the spec for other quoted names: the remaining ones ("data-x", "$encodeForHtmlAttribute", "$URLRewriting") are struct-literal keys, which Adobe accepts. None are named arguments in a call.

@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 final verdict @ d857ffe: APPROVE (matrix read leg by leg, adobe2023 explicit)

Matrix 37122451378 on head d857ffe8f: the run is a success, with 38 jobs passing and 1 skipped (notify). I read every leg's JUnit:

adobe2023 leg testcases failed differential spec (3 tests)
cockroachdb 6607 0 3 passed
mysql 6590 0 3 passed
oracle 6590 0 3 passed
postgres 6590 0 3 passed
sqlite 6600 0 3 passed
sqlserver 6590 0 3 passed

The previously zeroed adobe2023 leg is healthy: full test counts on every database, and linkToUrlForDifferentialSpec compiles and passes there. The other 26 engine × DB legs (adobe2025 ×6, boxlang ×6, lucee6 ×7, lucee7 ×7) are also 0 failed, with 6590-6607 tests each and the differential spec 3/3 everywhere. The RustCFML lane is 6517 passed, 0 failed, 0 errors (83 skipped).

Code verdict unchanged from my reviews of W1, W2 and W3. The PR stays a draft for orch1's 4.3 timing.

bpamiri added a commit that referenced this pull request Oct 3, 2026
…t 16c

rev1-r3 noted the only measured case for 16c was a single-expression arrow,
while the entry claimed "any direct call" -- and Adobe traps can be
closure-sensitive (16b-ext). Probed the quoted-named-arg call on adobe2023
(62023) in four isolated directory scopes:

  single-expression arrow        -> compile FAIL
  block-bodied arrow             -> compile FAIL
  regular function() closure     -> compile FAIL
  plain statement in a method    -> compile FAIL

So the trap is context-independent, not closure-sensitive -- "any direct call"
is correct. Controls confirm the boundaries: an unquoted named arg, a quoted
$-key struct literal, and the argumentCollection workaround all compile on
adobe2023. Reworded both entries to state the measured contexts and boundaries
explicitly.

Refs #4192

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
bpamiri added a commit that referenced this pull request Oct 3, 2026
… 2023) (#4199)

* docs(reference): document invariant 16c (quoted named-arg call, Adobe 2023)

A named argument whose name is a quoted string literal in a direct call --
obj.method("name" = value) -- is a compile error on Adobe CF 2023 only
(MissingNameException), while Lucee 6/7, BoxLang and Adobe CF 2025 all accept
it. This is the inverse footprint of its siblings: 16a fails both Adobe
engines, 16b/16b-ext are Adobe 2025 only, and this one is the lone 2023-only
shape -- so an Adobe 2025 smoke or Lucee-only local run does not cover it.

Add the shape, the verified-engine boundaries, the argumentCollection
workaround, and the 62023 bisect probe to both CLAUDE.md (invariant 16c) and
.ai/wheels/cross-engine-compatibility.md.

Surfaced by linkToUrlForDifferentialSpec (#4192), which zeroed every adobe2023
leg until the quoted named arg was moved onto a struct key.

Refs #4192

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(reference): record measured call-context boundaries for invariant 16c

rev1-r3 noted the only measured case for 16c was a single-expression arrow,
while the entry claimed "any direct call" -- and Adobe traps can be
closure-sensitive (16b-ext). Probed the quoted-named-arg call on adobe2023
(62023) in four isolated directory scopes:

  single-expression arrow        -> compile FAIL
  block-bodied arrow             -> compile FAIL
  regular function() closure     -> compile FAIL
  plain statement in a method    -> compile FAIL

So the trap is context-independent, not closure-sensitive -- "any direct call"
is correct. Controls confirm the boundaries: an unquoted named arg, a quoted
$-key struct literal, and the argumentCollection workaround all compile on
adobe2023. Reworded both entries to state the measured contexts and boundaries
explicitly.

Refs #4192

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…r-cost-4151

Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri bpamiri added this to the v4.2.0 milestone Oct 5, 2026
@bpamiri
bpamiri marked this pull request as ready for review October 5, 2026 05:06
@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 re-review @ c3f688a: code APPROVE (final sign-off after matrix run 37266376316)

The delta since my final approval at d857ffe8f is two commits: a merge of develop (0f34f4314, which brings in #4183's router index) and a changelog fragment.

  • This PR's own code is unchanged. I diffed global/, view/ and the differential spec against each side's merge base (2b91e4ce2 for d857ffe8f, and fecea1fea for this head). Both give the same 4 files (+293/−7), and the added and removed lines hash identically. routing.cfm's diff against develop is only the two URL-generation hunks: the placeholder find/replace in the substitution helper and the $argsResolved branch in URLFor. It doesn't touch perf(router): index dynamic routes by method and first two path segments #4183's reload bump of routeTableGeneration / dynamicRouteIndex (develop routing.cfm L445–450) or anything in Dispatch.cfc.
  • perf(router): index dynamic routes by method and first two path segments #4183 compatibility (the open point from my 30e712f9b review). The differential spec saves and restores routes, staticRoutes and namedRoutePositions, but not perf(router): index dynamic routes by method and first two path segments #4183's dynamicRouteIndex, and it doesn't need to:
    • The spec only generates URLs (linkTo / URLFor), and URL generation never reads the dynamic index. Only $findMatchingRoute does.
    • $dynamicRouteIndex() reuses its cache only while the generation, the route count and a sampled table signature all match. Its docblock calls out exactly this case, a table assigned straight to application.wheels.routes as specs do on restore. Building the test table also bumps the generation through $addRoute(). So the first match after this spec rebuilds the index against the restored table instead of using a stale one.
  • Changelog fragment (4151-linkto-urlfor-overhead.performance.md): it describes the three changes accurately, and correctly says the output is byte-for-byte unchanged.

@github-actions github-actions Bot added the docs label Oct 5, 2026
@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ c3f688a: APPROVE (final; matrix run 37266376316 read leg by leg)

  • Matrix run 37266376316 is on this head, and all 32 legs plus rustcfml succeeded. From the junit artifact for each leg:
    • 6773–6790 tests per leg, with 0 failures and 0 errors on every leg.
    • Three specs ran and passed on every leg, none skipped, including all six Adobe 2023 and all six Adobe 2025 legs and BoxLang:
  • PR CI on this head: 23 checks pass and 2 are skipped.

@bpamiri
bpamiri merged commit 7080e70 into develop Oct 5, 2026
64 checks passed
@bpamiri
bpamiri deleted the peter/linkto-urlfor-cost-4151 branch October 5, 2026 05:55
bpamiri added a commit that referenced this pull request Oct 5, 2026
…iew specs) (#4202)

* test(wheelstest): add shared $snapshotRoutes/$restoreRoutes/$clearRoutes helpers (#4194)

Route specs that redefine the table must restore it in afterEach or they leak
stale state into later specs. routes, staticRoutes, namedRoutePositions, the
urlFor caches (application- AND request-scoped) and #4183's router index are
separate structures, so a partial restore is the #4192 bug class (and
formsSpec already restored only routes + namedRoutePositions, dropping
staticRoutes).

Add three helpers to wheels.WheelsTest:
- $snapshotRoutes() -> deep-copy snapshot of every route-derived structure
  present (#4183 fields behind StructKeyExists, so no rebase needed);
- $restoreRoutes(snapshot) -> exact restore, deleting managed keys that were
  absent at snapshot time;
- $clearRoutes() -> empty the table + invalidate every route cache (shared,
  replacing the per-spec shims).

PR1 of a staged migration: helpers + a round-trip self-test
(SnapshotRoutesSpec) + migrate the view/* specs (links, urls,
paginationHelpers, forms). dispatch/mapper/global/middleware/security/hardener
follow in area PRs. Design: shared-docs state/dev1-design/evidence/4194-*.

Updates the CLAUDE.md / consumer-ai "Test-specific gotchas" line (and the app
template) that said $clearRoutes is not inherited -- it now is.

Verified locally, view 605/0/0 + wheelstest 267/0/0 on lucee7, adobe2023 AND
adobe2025.

Refs #4194
Refs #4183

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(test): move route-state helper idiom to consumer .ai/testing.md

Keep docs/consumer-ai/CLAUDE.md under its 8192-byte budget: replace the
inline $snapshotRoutes()/$restoreRoutes()/$clearRoutes() worked example
with a one-line pointer and move the full idiom to .ai/testing.md. Re-sync
the cli/lucli/templates/app copies so the consumer-docs check stays green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
bpamiri added a commit that referenced this pull request Oct 5, 2026
…#4203)

* test(wheelstest): add shared $snapshotRoutes/$restoreRoutes/$clearRoutes helpers (#4194)

Route specs that redefine the table must restore it in afterEach or they leak
stale state into later specs. routes, staticRoutes, namedRoutePositions, the
urlFor caches (application- AND request-scoped) and #4183's router index are
separate structures, so a partial restore is the #4192 bug class (and
formsSpec already restored only routes + namedRoutePositions, dropping
staticRoutes).

Add three helpers to wheels.WheelsTest:
- $snapshotRoutes() -> deep-copy snapshot of every route-derived structure
  present (#4183 fields behind StructKeyExists, so no rebase needed);
- $restoreRoutes(snapshot) -> exact restore, deleting managed keys that were
  absent at snapshot time;
- $clearRoutes() -> empty the table + invalidate every route cache (shared,
  replacing the per-spec shims).

PR1 of a staged migration: helpers + a round-trip self-test
(SnapshotRoutesSpec) + migrate the view/* specs (links, urls,
paginationHelpers, forms). dispatch/mapper/global/middleware/security/hardener
follow in area PRs. Design: shared-docs state/dev1-design/evidence/4194-*.

Updates the CLAUDE.md / consumer-ai "Test-specific gotchas" line (and the app
template) that said $clearRoutes is not inherited -- it now is.

Verified locally, view 605/0/0 + wheelstest 267/0/0 on lucee7, adobe2023 AND
adobe2025.

Refs #4194
Refs #4183

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

* test(mapper): migrate mapper-area specs to the shared route helpers (#4194 PR2)

Replace each spec's hand-rolled route save/restore with $snapshotRoutes() /
$restoreRoutes() and drop the local $clearRoutes shims (now inherited from
wheels.WheelsTest, added in #4202). Several of these restored only a subset --
mapperSpec, mapperModernSpec, mapper/MapperSpec, mapper/MapperHardenerShouldSpec,
mapper/MapperNullPatternSpec and mapper/MapperRobustnessSpec each dropped
namedRoutePositions and/or staticRoutes -- so the shared helper also closes
those latent leaks.

PR2 of the staged migration; stacked on #4202 (the helper is not on develop
yet). Depends on #4202; merge order #4202 then this.

Verified locally: full core suite clean on lucee7 (6518 pass; the only
failures are the known TestClient port-mapped loopback artifacts, 0 real),
adobe2023 (6532/0/0) and adobe2025 (6532/0/0); 13 mapper bundles present.

Refs #4194

Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(wheelstest): address review nits on route-state helper docs

- WheelsTest.cfc docblock: drop the stale 'once it merges' wording now that
  #4183 has landed; dynamicRouteIndex / routeTableGeneration are present.
- .ai/testing.md snippet: define 'var g = application.wo;' so the beforeEach
  example compiles as written.
- .ai/testing.md prose: name the dynamic route index and route-table
  generation in the capture list (the helper already captures both).
- Re-sync the cli/lucli/templates/app copy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
bpamiri added a commit that referenced this pull request Oct 5, 2026
…ute helpers (PR3) (#4204)

* test(wheelstest): add shared $snapshotRoutes/$restoreRoutes/$clearRoutes helpers (#4194)

Route specs that redefine the table must restore it in afterEach or they leak
stale state into later specs. routes, staticRoutes, namedRoutePositions, the
urlFor caches (application- AND request-scoped) and #4183's router index are
separate structures, so a partial restore is the #4192 bug class (and
formsSpec already restored only routes + namedRoutePositions, dropping
staticRoutes).

Add three helpers to wheels.WheelsTest:
- $snapshotRoutes() -> deep-copy snapshot of every route-derived structure
  present (#4183 fields behind StructKeyExists, so no rebase needed);
- $restoreRoutes(snapshot) -> exact restore, deleting managed keys that were
  absent at snapshot time;
- $clearRoutes() -> empty the table + invalidate every route cache (shared,
  replacing the per-spec shims).

PR1 of a staged migration: helpers + a round-trip self-test
(SnapshotRoutesSpec) + migrate the view/* specs (links, urls,
paginationHelpers, forms). dispatch/mapper/global/middleware/security/hardener
follow in area PRs. Design: shared-docs state/dev1-design/evidence/4194-*.

Updates the CLAUDE.md / consumer-ai "Test-specific gotchas" line (and the app
template) that said $clearRoutes is not inherited -- it now is.

Verified locally, view 605/0/0 + wheelstest 267/0/0 on lucee7, adobe2023 AND
adobe2025.

Refs #4194
Refs #4183

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

* test(mapper): migrate mapper-area specs to the shared route helpers (#4194 PR2)

Replace each spec's hand-rolled route save/restore with $snapshotRoutes() /
$restoreRoutes() and drop the local $clearRoutes shims (now inherited from
wheels.WheelsTest, added in #4202). Several of these restored only a subset --
mapperSpec, mapperModernSpec, mapper/MapperSpec, mapper/MapperHardenerShouldSpec,
mapper/MapperNullPatternSpec and mapper/MapperRobustnessSpec each dropped
namedRoutePositions and/or staticRoutes -- so the shared helper also closes
those latent leaks.

PR2 of the staged migration; stacked on #4202 (the helper is not on develop
yet). Depends on #4202; merge order #4202 then this.

Verified locally: full core suite clean on lucee7 (6518 pass; the only
failures are the known TestClient port-mapped loopback artifacts, 0 real),
adobe2023 (6532/0/0) and adobe2025 (6532/0/0); 13 mapper bundles present.

Refs #4194

Signed-off-by: Peter Amiri <peter@alurium.com>

* test(dispatch,global,security): migrate leak-class specs to shared route helpers (#4194 PR3)

Migrate the remaining partial-clear / leak-class route specs onto
$snapshotRoutes() / $restoreRoutes() and drop their local $clearRoutes shims:

- dispatch/findMatchingRouteMegaSpec (partial-clear shim)
- global/urlforSpec (partial-clear shim; keeps its URLRewriting + obfuscateUrls
  saves, which are not route-table state)
- security/PaginationXssSpec (never saved staticRoutes -- the formsSpec leak
  class; its $clearRoutes CALL stays, now inherited)

PR3 of the staged migration; stacked on #4203 (helper chain not on develop
yet). Depends on #4203 (and #4202); merge order #4202, #4203, then this.

Verified locally, full core suite clean on lucee7 (6518 pass; only the known
TestClient port-mapped loopback artifacts, 0 real), adobe2023 (6532/0/0) and
adobe2025 (6532/0/0).

Refs #4194

Signed-off-by: Peter Amiri <peter@alurium.com>

* test(global): keep urlForCache present before the route-state snapshot

Migrating urlforSpec onto the shared $snapshotRoutes/$restoreRoutes helpers
brought urlForCache into the restored key set. urlForCache is a lazily-created
persistent app-scope memo (routing.cfm), so when it was absent at snapshot time
$restoreRoutes deleted it on the afterEach — cascading so every test ended with
it gone. The issue-2955 'memo lives in application scope so it survives across
requests' test then found the memo absent (Expected [false] to be true).

Ensure urlForCache exists before the snapshot in the outer beforeEach, so the
full-state restore preserves the key instead of deleting it — matching the
framework's treatment of it as a persistent memo. All assertions are unchanged;
the issue-2955 block still manages the cache contents via its own save/restore.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant