Skip to content

perf: prepare each grant's BXL expressions once, not on every request - #6646

Merged
habdelra merged 9 commits into
mainfrom
cs-13772-policy-prepare-each-grants-bxl-expressions-once-not-on-every
Oct 12, 2026
Merged

habdelra merged 9 commits into
mainfrom
cs-13772-policy-prepare-each-grants-bxl-expressions-once-not-on-every

Conversation

@habdelra

@habdelra habdelra commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

In service of CS-13772, which makes an OperationGrant's BXL expressions cheap to evaluate on every request. A grant's actingUser, blocklist, rateLimitRequests and rateLimitWindowSeconds ran through the one-shot runBxlTransform. So every evaluation parsed and profile-checked a source that can't change while the compiled policy is held. The blocklist and the rate limit are settled at admission for every request from a caller who isn't signed in, so this cost landed on the hottest anonymous path.

evaluateGrantExpression now holds one prepareBxlTransform program per compiled grant expression, in a WeakMap keyed on the CompiledGrantExpression. This is the same arrangement the gate uses for where predicates. A program lives exactly as long as the compiled policy that holds it, and a recompiled policy prepares its own. An expression that fails to prepare is not held, and fails again on its next evaluation. One that throws on the values it reads, such as a realmConfig() key that is absent, is held: preparing it succeeded, and the next request's values may well be there.

The counter, grantExpressionPreparations(), counts for the whole process rather than per realm core. Admission settles traffic from the realm's anonymous admission environment, which has no operation core to count against. Tests read the counter as a delta.

🤖 Generated with Claude Code

habdelra and others added 3 commits October 11, 2026 09:43
…-flag-acting-user-key-and-realmjson' into cs-13772-policy-prepare-each-grants-bxl-expressions-once-not-on-every
…te-each-predicate-once-not-on-every' into cs-13772-policy-prepare-each-grants-bxl-expressions-once-not-on-every

# Conflicts:
#	packages/runtime-common/card-operations/gate.ts
#	packages/runtime-common/card-operations/transforms.ts
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T13:55:16.732340Z 2a70eb7 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@habdelra habdelra left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] This review checked whether the compiled grant expressions keep their identity from one request to the next on the admission, explain and actingUser paths, whether moving from runBxlTransform to a held program changes error phases or context slots, whether the new tests could pass without the cache, and the resolution in the integration base where policy was added to TransformProgramContext and program.run.

No blocking issues. The admission and actingUser paths reach the cached CompiledRealmPolicy's own grant objects. Explain holds one compile for each explain, so draft explains prepare once for each draft and release the programs with it. The base merge carries policy through, and bxl-mirror-check.ts checks the key sets in both directions, so a dropped slot would fail the typecheck.

  1. Non-blocking: the failing-expression half of the new unit test never checks that the first settlement failed (inline on anonymous-access-test.ts).


let failing = compiled('blocklist', 'realmConfig("blockedIps")');
let beforeFailing = grantExpressionPreparations();
await settleTraffic(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Please assert on this first settlement. The result is discarded, so the assertion below, "an expression that throws on its values is still prepared once", never checks that the expression threw. If realmConfig("blockedIps") against {} ever produced a value instead of throwing, this half would pass while testing nothing about failed runs keeping their program. That is the case the comment on preparedExpressions and the description call out.

let unset = await settleTraffic(
  { blocklist: failing },
  { realmConfig: {} },
  PLATFORM,
);
assert.true(blocklistCloses(unset), 'the unset blocklist fails');

Asserting unset.blocklist.failed names the evaluate phase would also pin that the failure happens at run time rather than at preparation. Non-blocking: a test gap in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Fixed in 0645e61. The first settlement is now kept and asserted to have failed with BxlTransformError (evaluate), so the 'still prepared once' check covers a program that is held after it throws at run time.

habdelra and others added 5 commits October 11, 2026 09:57
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-flag-acting-user-key-and-realmjson' into cs-13772-base-6622-and-6645
…re-each-grants-bxl-expressions-once-not-on-every
…-flag-acting-user-key-and-realmjson' into cs-13772-base-6622-and-6645
…re-each-grants-bxl-expressions-once-not-on-every
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Host Test Results

  1 files  ±    0    1 suites  ±0   7m 7s ⏱️ - 1h 31m 56s
295 tests  - 5 107  295 ✅  - 5 096  0 💤  - 11  0 ❌ ±0 
295 runs   - 5 122  295 ✅  - 5 111  0 💤  - 11  0 ❌ ±0 

Results for commit 7e7f9cb. ± Comparison against earlier commit f934873.

Realm Server Test Results

    1 files  ± 0    319 suites  +1   2h 0m 19s ⏱️ -17s
5 157 tests +23  5 157 ✅ +23  0 💤 ±0  0 ❌ ±0 
5 216 runs  +23  5 216 ✅ +23  0 💤 ±0  0 ❌ ±0 

Results for commit 7e7f9cb. ± Comparison against earlier commit f934873.

@habdelra
habdelra requested a review from a team October 11, 2026 18:33

@lukemelia lukemelia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the compilation performance hit? Are we OK incurring it on every instance of the realm-server tier?

@habdelra
habdelra changed the base branch from cs-13772-base-6622-and-6645 to main October 12, 2026 19:52
…re-each-grants-bxl-expressions-once-not-on-every

# Conflicts:
#	packages/runtime-common/card-operations/grant-expressions.ts
@habdelra

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] @lukemelia This PR doesn't add compilation. It moves it out of the request path. Before this change, every evaluation went through runBxlTransform, which is prepareBxlTransform(...).run(...). So every anonymous request paid the parse and profile check for each grant expression it settled. Now each realm-server instance pays that once per expression, for each compiled policy it holds. The preparation is lazy: it happens on the first request that settles the expression, not at boot.

I measured @cardstack/bxl/transform on Node 24, with this PR's test expressions:

expression prepare run a held program runBxlTransform (old path, per request)
realmConfig("blockedIps") 15–30 µs 9 µs 22 µs
realmConfig("anonymousRequests") 15 µs 4 µs 25 µs
60 9 µs 1 µs 11 µs
policy().actingUser // "@bot:localhost" 25 µs 14 µs 29 µs

These are warm, in-process microbenchmarks, and the numbers are noisy. The very first preparation in a fresh process took about 5 ms. That is module and JIT warm-up, which the old path also paid on its first request.

So on each instance a grant costs tens of microseconds, once for each of its four expressions, plus a few small parsed trees held in memory. The WeakMap releases them when the compiled policy is replaced. Every request after the first saves roughly 10–20 µs per expression it settles. The per-instance cost is a one-time version of a cost every instance was already paying on every request.

@habdelra
habdelra merged commit c2c85f8 into main Oct 12, 2026
105 of 107 checks passed
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.

2 participants