You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Look up only a runtime class's own witnesses - #5496
A runtime class's first interface lookup collected the witness rules of every runtime class implementing that interface, then tested each one against the receiver. Minted classes stay in the witness table until a collection sweeps them, so every new class was slower than the one before.
// each iteration slower than the last, until a GC sweeps old classes
for (let i = 0; i < 4000; i += 1) {
letminted=reflect.Class.new(
"Row"+i.to_string(),
{ "label": string.type() },
implementations= [reflect.Interface.implementation<Labeled>().field("label")],
);
lettypeRow=minted;
letrow: Row=baml.json.to<Row>({ "label": "r" });
shout_of(row) // `match (value) { let labeled: Labeled => ... }`
}
A witness's for pattern is always exactly its class, so it can only apply to a receiver of that class. The dynamic table is now keyed by interface and then by class, and a lookup for a class receiver reads that class's rules only. Any other receiver still gets every witness, as before.
The loop above, ms per batch of 500 classes:
batches 1–8
total
canary
38, 96, 159, 229, 301, 339, 385, 496
2,043 ms
this PR
8, 6, 7, 6, 6, 6, 7, 6
52 ms
The interfaces::mint classes with impls 200 workload from #5421 comes back with jobs = 60, now that a job's time no longer depends on the jobs before it.
workload
canary
this PR
change
interfaces::mint-classes-with-impls-200, 60 jobs
99.4 ms
7.7 ms
−92%
same, 1 job
22.9 ms
8.4 ms
−63%
interfaces::polymorphic-dispatch-100k (control)
52.4 ms
52.7 ms
≈
interfaces::to-string-interpolation-10k (control)
28.2 ms
27.4 ms
≈
interfaces::generic-from-json-20k (control)
48.8 ms
48.0 ms
≈
ms per job, speedtest VM row with telemetry off, release builds, pinned to one CPU, two interleaved rounds of 3 trials (the 1-job row: 5 trials).
Registering a witness still bumps the engine-wide generation, which drops every VM's impl cache each time a class is minted. That is a separate cost and is left alone here.
Fixes B-2000.
RUN_CODSPEED=1
Summary by CodeRabbit
Bug Fixes
Interface implementation lookup now selects class-specific implementations for class receivers, preventing implementations registered for other classes from affecting dispatch. Non-class receivers continue to consider implementations across the interface.
Tests
Added a benchmark workload combining dynamically created classes, interface methods, typed order decoding, enum comparisons, and string interpolation.
A runtime class's first interface lookup collected the witness rules of
every runtime class that implements the interface, then tested each one
against the receiver. Classes stay in the witness table until a collection
sweeps them, so each new class was slower than the last: 500 classes took
38 ms, the 4,000th batch of 500 took 496 ms.
A witness's `for` pattern is always exactly its class, so it can only
apply to a receiver of that class. The table is now keyed by interface,
then by class, and a lookup for a class receiver reads that class's rules
only. Other receivers still get every witness, as before.
The `mint classes with impls 200` workload from #5421 comes back with a
normal job count, now that a job's time no longer depends on the jobs
before it.
No actionable comments were generated in the recent review.
Walkthrough
The VM now indexes dynamic implementation witnesses by interface and runtime class. The resolver uses receiver-specific witnesses for class receivers. A new speedtest workload exercises minted classes with implementations and typed order decoding in BAML, JavaScript, and Python.
Changes: Dynamic witness resolution
Layer / File(s)
Summary
Nested witness index baml_language/crates/bex_vm/src/package_load.rs
DynDispatchTables now groups weak rule pointers by interface and class. rules_of_class returns class-scoped rules, while rules_of still returns all rules for an interface. Garbage-collection traversal and tests now cover the nested index.
rules_for accepts the receiver type. Class receivers use witnesses registered for that class; other receiver types use all witnesses for the interface. Resolver call sites pass the receiver for coherence checks, hash eligibility, implementation selection, and recursive membership proofs.
Adds a workload in BAML, JavaScript, and Python that creates dynamic classes with Labeled implementations and processes serialized orders. The benchmark configuration sets expect to 9415, jobs to 60, and warmup to 1.
BoundaryML/baml#5421: Registers minted witnesses with their classes and changes the same witness table.
BoundaryML/baml#5418: Changes resolver candidate handling and dynamic witness lookup.
BoundaryML/baml#5423: Changes dynamic witness entries and visitor-based table maintenance.
Suggested reviewers:2kai2kai2, aaronvg
Merge Risk: Minimal · up to 98826
No merge-blocking issue was identified in the witness lookup change or benchmark.
Security architecture risk: Low · up to 98826
The change narrows implementation lookup without an identified expansion of access or authority. Class ownership and implementation checks remain intact. Remaining uncertainty concerns concurrent execution and interruption recovery, rather than a demonstrated security defect.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The demonstrated scope is implementation resolution across VMs sharing one engine's registry and generation. That engine-local scope is not evidence of tenant isolation; deployment-level engine sharing and tenant exposure are not supplied.
Trust Boundaries and Controls
inferred — For program-created classes, supplied witness definitions become exact-class patterns and undergo coherence checking before publication. Subsequent resolution still validates applicability. The inspected change therefore does not establish a path for one class to acquire another class's implementation through a shared name or field shape.
Resilience and Maintainability Implications
observed — The class remains the strong owner of its witness rules; the dispatch table remains a weak findability index. Registration publishes entries under a write lock and then advances the shared resolution generation. Source tests additionally check that rooting only a class preserves its witness through collection and that dropping the class removes the indexed witness.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: runtime lookup now uses only witnesses belonging to the receiving class.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 14 functions across 4 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Warning
Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.
A rabbit checks the class-bound trail And finds the witness without fail Orders tumble through the run Two hundred rounds beneath the sun The carrot counter marks the end
CodeRabbit Verify
Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: f5569589-b5b4-49e1-801a-b42b5a8e5b19
Commits
Reviewing files that changed from the base of the PR and between 857c3f2 and 988267b.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
⚠️ Please fix the size gate issues or acknowledge them by updating baselines.
Artifact
Platform
File
Gzip
Gated on
Baseline
Delta
Status
✅
baml-cli
Linux
🔒 64.5 MB
27.4 MB
file
73.1 MB
-8.6 MB (-11.8%)
OK
❌
packed-program
Linux
🔒 39.2 MB
16.1 MB
file
29.2 MB
+10.0 MB (+34.5%)
FAIL
❌
bridge_wasm
WASM
25.8 MB
🔒 7.6 MB
gzip
5.7 MB
+2.0 MB (+34.9%)
FAIL
🔒 = the size this artifact is GATED on (ceiling + delta). Binaries gate on file size (installed binary); WASM gates on gzip (download size). The other size is shown for information only.
Details & how to fix
Violations:
packed-program (Linux) file_bytes: 39.2 MB exceeds limit of 30.1 MB (exceeded by +9.1 MB, policy: max_file_bytes)
packed-program (Linux) file_delta_pct: +34.5% exceeds limit of 3.0% (exceeded by +31.5pp, policy: max_delta_pct)
bridge_wasm (WASM) gzip_bytes: 7.6 MB exceeds limit of 5.9 MB (exceeded by +1.8 MB, policy: max_gzip_bytes)
bridge_wasm (WASM) gzip_delta_pct: +34.9% exceeds limit of 3.0% (exceeded by +31.9pp, policy: max_delta_pct)
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🤖 Claude on behalf of Antonio
A runtime class's first interface lookup collected the witness rules of every runtime class implementing that interface, then tested each one against the receiver. Minted classes stay in the witness table until a collection sweeps them, so every new class was slower than the one before.
A witness's
forpattern is always exactly its class, so it can only apply to a receiver of that class. The dynamic table is now keyed by interface and then by class, and a lookup for a class receiver reads that class's rules only. Any other receiver still gets every witness, as before.The loop above, ms per batch of 500 classes:
The
interfaces::mint classes with impls 200workload from #5421 comes back withjobs = 60, now that a job's time no longer depends on the jobs before it.interfaces::mint-classes-with-impls-200, 60 jobsinterfaces::polymorphic-dispatch-100k(control)interfaces::to-string-interpolation-10k(control)interfaces::generic-from-json-20k(control)ms per job, speedtest VM row with telemetry off, release builds, pinned to one CPU, two interleaved rounds of 3 trials (the 1-job row: 5 trials).
Registering a witness still bumps the engine-wide generation, which drops every VM's impl cache each time a class is minted. That is a separate cost and is left alone here.
Fixes B-2000.
RUN_CODSPEED=1
Summary by CodeRabbit