perf(stdlib): cherry-pick stdlib features per program — drop the crypto force, split compression per codec - #7029
Merged
Conversation
jdalton
added a commit
to jdalton/perry
that referenced
this pull request
Jul 29, 2026
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR refines optimized stdlib feature selection, adds codec-specific compression gating and extension-level Zstandard support, implements ChangesRuntime compilation and stdlib optimization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Program
participant HIRFeatureDetection
participant OptimizedLibsDriver
participant CargoWorkspace
Program->>HIRFeatureDetection: compile module HIR
HIRFeatureDetection->>OptimizedLibsDriver: detected codec and crypto usage
OptimizedLibsDriver->>CargoWorkspace: filtered feature set
CargoWorkspace-->>OptimizedLibsDriver: optimized library build result
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
.github/workflows/security-audit.yml (1)
128-137: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPinned SkillSpector SHA is duplicated here with no parity gate against
external-tools.json.The comment states this rides "the same git SHA as external-tools.json tools.skillspector", but nothing enforces it — bumping the manifest silently leaves CI scanning the old revision. This is exactly the drift
checkDockerPrebakeguards against for the Docker prebake; either derive the SHA from the manifest at run time or add a parity assertion tocheckPins.♻️ Derive it from the manifest instead
- name: Scan each skill run: | set -euo pipefail + SHA="$(node -e 'const j=require("./external-tools.json");process.stdout.write(j.tools.skillspector.version)')" for skill in .claude/skills/*/; do echo "::group::skillspector ${skill}" uv tool run --python 3.12 \ - --from git+https://github.com/NVIDIA/skillspector@2eb84478 \ + --from "git+https://github.com/NVIDIA/skillspector@${SHA}" \ skillspector scan "${skill}" --no-llm echo "::endgroup::" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/security-audit.yml around lines 128 - 137, Update the “Scan each skill” workflow step to derive the SkillSpector git revision from the tools.skillspector entry in external-tools.json instead of hardcoding the SHA, while preserving the existing skillspector scan invocation and pinned-revision behavior.scripts/soak/external-tools.mts (3)
151-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated "expired and prunable" predicate across
staleBypassesandpruneExpiredSoakBypasses.Both functions repeat the same three-step validity/arithmetic/expiry test. Since the two must agree (a divergence would make the warning and the pruner disagree about what has cleared), extract a single predicate.
♻️ Proposed refactor
+function isExpiredBypass(bypass: { published: string; removable: string }): boolean { + if (!isValidIsoDate(bypass.published) || !isValidIsoDate(bypass.removable)) { + return false + } + // Wrong-arithmetic annotations are a hard checkPins failure, never + // stale/prunable — a too-early removable must not read as "cleared". + if (bypass.removable !== addDaysIso(bypass.published, SOAK_DAYS)) { + return false + } + return bypass.removable < todayIso() +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/soak/external-tools.mts` around lines 151 - 205, Extract the shared valid-and-expired soak bypass check from staleBypasses and pruneExpiredSoakBypasses into a single predicate, then reuse it in both functions. Preserve the existing behavior for missing, malformed, wrong-arithmetic, and non-expired annotations so warning and pruning remain consistent.
268-271: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTrailing-space substring match is the same brittleness the arg-list parse above fixed.
toolchain install ${channel}false-fails when the channel is the last token on the line (RUN rustup toolchain install nightly-2026-07-04).installedToolchainson Line 232 already has the tokens — though it filters to/^\d/, so widen it or parse channels the same way.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/soak/external-tools.mts` around lines 268 - 271, Update the toolchain preinstallation check near the channel extraction to use parsed install-command tokens instead of the trailing-space substring `toolchain install ${channel} `. Reuse or broaden the existing `installedToolchains` parsing so channels such as `nightly-2026-07-04` are detected even when they are the final token on a line, while preserving the warning for genuinely missing pinned toolchains.
611-618: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider scoping
SFW_UNKNOWN_HOST_ACTION=ignoreto interactive/dev use.Every shimmed install — including CI, where the firewall is the actual gate — now runs with unknown-host blocking disabled. Gating on
CI(or an opt-in env var) preserves the dev ergonomics without softening the CI posture.♻️ Suggested shape
-export SFW_UNKNOWN_HOST_ACTION=ignore +# Dev-only relaxation; CI keeps the enterprise default (block). +if [ -z "\${CI:-}" ]; then + export SFW_UNKNOWN_HOST_ACTION=ignore +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/soak/external-tools.mts` around lines 611 - 618, Scope the SFW_UNKNOWN_HOST_ACTION override in the external-tools shim to interactive/development usage instead of exporting it unconditionally. Preserve the existing CI firewall behavior by gating the ignore value on CI status or an explicit development opt-in, while keeping the sentinel export and exec sfw flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-codegen/src/expr/this_super_call.rs`:
- Around line 713-743: Extend the Expr::SuperCallSpread handling with a
DOMException branch matching the existing parent_name == "DOMException" logic in
Expr::SuperCall. Lower the spread arguments, read the first two array elements
with undefined defaults, invoke js_dom_exception_subclass_init for this, apply
SelfOnly field initializers via apply_field_initializers_recursive, and return
undefined.
In `@crates/perry-ext-zlib/src/stream.rs`:
- Line 421: Update the CodecState::ZstdDec branch to flush the decoder with
w.flush()? before calling w.into_inner(), preserving error propagation and
ensuring pending decoded bytes are written before ownership is returned.
In `@crates/perry-runtime/src/event_target.rs`:
- Around line 337-360: Update js_dom_exception_subclass_init to root this_value,
message, and name in RuntimeHandleScopes before any coercion or allocation.
After each allocating operation, reload the corresponding boxed value from its
handle before deriving or using raw pointers, including before each
set_event_field call, so exception, message_ptr, and name_ptr are never reused
across allocations.
In `@scripts/soak/soak.mts`:
- Around line 401-420: The checkTazeConfig function must validate that
maturityPeriod is set specifically to the imported SOAK_DAYS binding, rather
than checking maturityPeriod and constants.mts independently. Match the relevant
import and maturityPeriod: SOAK_DAYS expression together, and report the
existing finding when that combined configuration is absent.
- Around line 434-491: Remove the SOAK_DAYS === 0 early returns from
checkDependabotCooldown and fixDependabotCooldown. Continue validating every
Dependabot block and rewrite existing default-days values to 0 so the documented
opt-out disables the surface while preserving missing-block findings.
In `@scripts/soak/soak.test.mts`:
- Around line 6-54: Update the soak test fixtures to derive all valid durations
and date offsets from the canonical constants, importing SOAK_MINUTES alongside
SOAK_DAYS and using it for minute-based values and addDaysIso-derived dates.
Replace hardcoded valid 7-day, 10080-minute, and equivalent date values
throughout the referenced tests, while retaining fixed literals only for
intentionally invalid or drifted inputs; update the affected fixtures around
CLEAN_YAML and the tests covering lines 81-91, 104-146, 163-183, and 210-213.
In `@scripts/soak/update-deps.mts`:
- Around line 59-116: The updateCargo function currently permits a mutating
cargo update when min-publish-age is unsupported and only warns afterward.
Before running the non-dry-run update, detect whether the selected cargo
resolver enforces publish age and fail with a nonzero status if unavailable;
allow dry runs to retain diagnostic behavior, and preserve the existing
blocked-age and unsupported-toolchain reporting for cases that occur after
execution.
---
Nitpick comments:
In @.github/workflows/security-audit.yml:
- Around line 128-137: Update the “Scan each skill” workflow step to derive the
SkillSpector git revision from the tools.skillspector entry in
external-tools.json instead of hardcoding the SHA, while preserving the existing
skillspector scan invocation and pinned-revision behavior.
In `@scripts/soak/external-tools.mts`:
- Around line 151-205: Extract the shared valid-and-expired soak bypass check
from staleBypasses and pruneExpiredSoakBypasses into a single predicate, then
reuse it in both functions. Preserve the existing behavior for missing,
malformed, wrong-arithmetic, and non-expired annotations so warning and pruning
remain consistent.
- Around line 268-271: Update the toolchain preinstallation check near the
channel extraction to use parsed install-command tokens instead of the
trailing-space substring `toolchain install ${channel} `. Reuse or broaden the
existing `installedToolchains` parsing so channels such as `nightly-2026-07-04`
are detected even when they are the final token on a line, while preserving the
warning for genuinely missing pinned toolchains.
- Around line 611-618: Scope the SFW_UNKNOWN_HOST_ACTION override in the
external-tools shim to interactive/development usage instead of exporting it
unconditionally. Preserve the existing CI firewall behavior by gating the ignore
value on CI status or an explicit development opt-in, while keeping the sentinel
export and exec sfw flow unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d374643d-88bd-482d-96a0-afdbf11cb930
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktools/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
.cargo/config.toml.claude/skills/soak/SKILL.md.github/dependabot.yml.github/workflows/security-audit.yml.github/workflows/soak-autofix.yml.github/workflows/zizmor.yml.github/zizmor.yml.npmrcchangelog.d/6912-security-tooling-soak.mdchangelog.d/7021-real-npm-cli-compile-gaps.mdchangelog.d/7029-stdlib-cherry-pick.mdcrates/perry-codegen/src/expr/this_super_call.rscrates/perry-codegen/src/ext_registry.rscrates/perry-codegen/src/lower_call/new_helpers.rscrates/perry-codegen/src/runtime_decls/strings_part2.rscrates/perry-ext-zlib/Cargo.tomlcrates/perry-ext-zlib/src/stream.rscrates/perry-runtime/src/event_target.rscrates/perry-stdlib/Cargo.tomlcrates/perry-stdlib/src/common/async_bridge.rscrates/perry-stdlib/src/common/dispatch.rscrates/perry-stdlib/src/common/dispatch/fastify_net_zlib.rscrates/perry-stdlib/src/common/dispatch/init.rscrates/perry-stdlib/src/common/dispatch/method_dispatch.rscrates/perry-stdlib/src/common/dispatch/property_dispatch.rscrates/perry-stdlib/src/lib.rscrates/perry-stdlib/src/zlib.rscrates/perry/src/commands/compile/collect_modules/feature_detect.rscrates/perry/src/commands/compile/optimized_libs.rscrates/perry/src/commands/compile/optimized_libs/driver.rscrates/perry/src/commands/compile/optimized_libs/freshness.rscrates/perry/src/commands/compile/optimized_libs/tests.rscrates/perry/src/commands/compile/strip_dedup.rscrates/perry/src/commands/compile/types.rscrates/perry/src/commands/stdlib_features.rsexternal-tools.jsonpackage.jsonscripts/soak/constants.mtsscripts/soak/external-tools.mtsscripts/soak/external-tools.test.mtsscripts/soak/paths.mtsscripts/soak/soak.mtsscripts/soak/soak.test.mtsscripts/soak/update-deps.mtsscripts/soak/update-deps.test.mtstools/package.jsontools/pnpm-workspace.yamltools/taze.config.mts
This was referenced Jul 29, 2026
…to force, split compression per codec Two coarse inclusion points in the auto-optimize path made every stdlib-linking binary carry subsystems it never used: 1. build_optimized_libs force-inserted perry-stdlib's `crypto` feature for EVERY rebuild (a leftover from the old perry/updater → js_crypto_ed25519_verify extern). A program that only imports node:zlib linked RSA/EC/Ed25519/Ed448/ML-KEM/x509/JWT/bcrypt/argon2. The force is gone; crypto now joins only via the import mapping, the crypto-builtin/WebCrypto HIR gates, or a new js_crypto_*/js_webcrypto_* prefix net in perry-codegen's ext_registry (with an object-cache- replayable capture marker, PerryTS#6439 shape). An `async-runtime` floor stays forced: the always-on worker_threads/readline bridge needs it, exactly as the crypto force provided transitively before. 2. `compression` was monolithic (flate2+brotli+zstd). It now splits into compression-gzip (base, gates `pub mod zlib`), compression-brotli and compression-zstd (each implying the base); `compression` remains the union for backwards compat, so default / `full` / `--features compression` builds are byte-identical. `import 'node:zlib'` selects only compression-gzip; the Brotli/zstd backends are added when HIR usage detection sees a matching API token (ctx.uses_zlib_brotli / ctx.uses_zlib_zstd), or unconditionally when deferred dynamic code could name a codec at runtime. When the well-known flip routes node:zlib to perry-ext-zlib, the sub-features are dropped with the base so the ext crate's js_zlib_* symbols can't duplicate. Measured (macOS arm64, stripped, auto-optimize): gzip-only node:zlib program 9,629,736 B → 6,618,416 B (−31.3%) zlib+events mid-size program 9,662,968 B → 6,668,160 B (−31.0%) crypto.randomUUID() program unchanged (still links crypto) hello-world unchanged (runtime-only link) Attribution on the zlib program: −1.36 MB from the crypto force removal, −1.65 MB from the per-codec compression split. Feature strings feed auto_optimized_cache_key, so the new sets produce fresh build stamps automatically. cargo test -p perry --bin perry: 756 passed (3 new); perry-codegen ext_registry: 10 passed (2 new); perry-stdlib compiles standalone under each new sub-feature.
proggeramlug
force-pushed
the
perf/stdlib-cherry-pick
branch
from
July 30, 2026 07:07
def1653 to
7250b23
Compare
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
2 participants
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.
Makes stdlib inclusion granular so the auto-optimize path links only what a program uses. Defaults and the prebuilt/out-of-tree path are byte-identical; the wins land on per-program rebuilds.
Measured (macOS arm64, stripped, auto-optimize path)
node:zlibgzip-onlycrypto.randomUUID()sfw, 28 npm deps)Attribution on the zlib program: −1.36 MB from dropping the crypto force, −1.65 MB from the per-codec compression split.
What changed
cryptoforce is gone.driver.rsinsertedcryptointo every stdlib-linking build — RSA/EC/Ed25519/ML-KEM/x509/JWT/bcrypt/argon2 rode along in zlib-only programs. Selection is now three-layered: module imports (module_to_features), the existingctx.uses_crypto_builtinsHIR gates, and a new codegen safety net inext_registry.rs::record_ffi_call— any emittedjs_crypto_*/js_webcrypto_*call flips the feature, with a replayable capture marker so warm object caches reproduce the flip (same shape as auto-optimize: @effect/platform Socket.ts link fails on undefined js_ws_connect_start despite external-ws-pump enabled (links fine on plain runtime) #6439). Anasync-runtimefloor stays forced: the always-on worker_threads/readline bridge requires it (a truly bare stdlib never compiled — the crypto force had been masking that transitively).compressionsplit intocompression-gzip/compression-brotli/compression-zstd(umbrella kept as the union).import 'node:zlib'alone now selects gzip/deflate only; brotli/zstd are added by zero-false-negative API-token detection (incl.BROTLI_*/ZSTD_*constants), with a full-umbrella fallback when deferred dynamic-code sites exist (eval/new Function). When the well-known flip routesnode:zlibto perry-ext-zlib, sub-features drop with the base (they'd duplicatejs_zlib_*symbols).Splits evaluated and deliberately dropped
net/tls(well-known flip already routes to perry-ext-net),database-sqlite(already import-gated),http-client(already split by #5174), internal crypto hash-vs-asymmetric (invasive — shared util/dispatch; most of the win captured by removing the force; noted as follow-up —crypto.randomUUID()-only programs still pay ~1.6 MB).Validation
cargo test -p perry --bin perry: 756 passed, 0 failed (3 new); perry-codegen ext_registry 10 passed (2 new)crypto,async-runtime, and default/full--helpcorrect), 3% smallerKnown accepted risk
Fully dynamic codec access (
zlib[name]()with a runtime string, without eval) bypasses token detection → the codec dispatch returnsundefined. The dyn-eval fallback coverseval/new Function; static lowering is fully covered. Same class as the existinguses_regex-style gates. Cache stamps rotate automatically (feature strings feedauto_optimized_cache_key), and older checkouts driven by a newer binary drop unknown feature names via the #7021 skew guard.Summary by CodeRabbit
New Features
DOMException, including standard message, name, and code behavior.Performance
Bug Fixes