fix: #741 path + #742 tty + partial #740 class-expr (v0.5.896 + v0.5.897) - #749
Merged
Conversation
Contributor
Author
proggeramlug
force-pushed
the
worktree-fix-740-741-742
branch
from
May 13, 2026 09:28
902dcd0 to
e789dc4
Compare
…r (v0.5.901)
and toNamespacedPath added; new HIR variants + runtime helpers; 5-line
issue repro matches Node, parity diff drops from 15 to 2 lines.
bound-method closure so typeof reports "function"; process.std*.isTTY
returns undefined (not boolean false) when not a TTY, per Node spec;
parity test now matches Node byte-for-byte.
instead of an empty-arg New, so `const C = class {...}; new C(args)`
constructs with the supplied args. Standalone Effect repro moves from
"TypeError" to running through with undefined fields. Full fix needs
runtime constructor dispatch for class refs read from object fields.
….5.902)
Builds on v0.5.901's class-expr→ClassRef change. Two new shapes now
resolve a class-ref read out of an object-literal field back to the
underlying class:
const O = { Inner: class extends Base {…} };
new O.Inner(args) // direct
const C = O.Inner; new C(args) // through an intermediate let
Both share a new per-function side table FnCtx.local_class_field_aliases
populated when Stmt::Let sees `init = New { __AnonShape, args }` and
walks the class's field order against args — any Expr::ClassRef arg
becomes a (local_id, field_name) → class_name entry. The map is also
propagated through `let O2 = O`.
Effect DoD still blocked on runtime parent-constructor dispatch.
proggeramlug
force-pushed
the
worktree-fix-740-741-742
branch
from
May 13, 2026 10:05
e789dc4 to
48b5666
Compare
proggeramlug
added a commit
that referenced
this pull request
May 13, 2026
…ards) CI run on PR #750 surfaced a regression in test_memory_json_churn.ts: under all three GC profiles (default, mark-sweep, gen-gc+wb) the test went from PASS at ~120 MB pre-fix to FAIL at 461 MB (limit 250 MB). The churn test allocates ~13 KB per iteration across 5k iterations into a deliberately fragmented arena where every block holds both live and dead objects — so a GC cycle sweeps 91-95% of bytes dead but reclaims *zero* blocks, step-doubles on the productive sweep heuristic, raises the trigger, and the cascade compounds. The original bytes-bump correctly deferred GC indefinitely on that shape; my v0.5.900 fix made GC fire and exposed the fragmentation cascade. Revised the guards to gate on the suppressed window's actual arena growth and split tiny-parse vs medium-or-larger parse handling: (1) `GC_PRE_SUPPRESS_BYTES` — gc_suppress snapshots arena_total so gc_bump_malloc_trigger can compute parse_growth. (2) `GC_TRIGGER_BUMPED` — once-per-cycle flag, applied only when parse_growth >= 1 MB (the json_pipeline_full and json_polyglot shapes). Cleared at the top of gc_collect_inner so all collection entry points re-arm. Tiny parses (< 1 MB, the churn shape) bypass the flag and bump every call, which preserves the pre-fix deferred-GC behavior. (3) Step cap at `GC_THRESHOLD_INITIAL_BYTES` (64 MB) — bound the bump's effective step so post-73a48ced step-doubling can't grant hundreds of MB of headroom per bump. Validation after revision: benchmarks/json_polyglot (lazy-tape default): roundtrip peak RSS: 250 → 223 MB (-11%, was -14% under v1) field-access peak RSS: 407 → 305 MB (-25%, was -39% under v1) scripts/run_memory_stability_tests.sh: 18/18 PASS (was 15/18 PASS, 3/18 FAIL under v1) test_memory_json_churn lands at 209 MB (limit 250) Pipeline-full-style probe: ~135 ms, no mid-iterate GC, 217 MB — the json_pipeline_full optimization is preserved because the 108 MB parse blows past the 1 MB tiny-parse threshold and the once-per-cycle flag still allows the first bump. perry-runtime tests: 250/0/0. Versioning: rebased onto current main (v0.5.902 → v0.5.903) since #743 took v0.5.900 and #749 took v0.5.901/v0.5.902 while this PR was in CI. Updated changelog entry text to match the revised approach.
5 tasks
- cargo fmt collapsed a 2-line `let layout = …Layout::from_size_align(...)` in object.rs:7385 (lint job) - regen docs/api/perry.d.ts and docs/src/api/reference.md for the new path.matchesGlob / path.toNamespacedPath entries added in v0.5.901 (api-docs-drift job)
proggeramlug
added a commit
that referenced
this pull request
May 13, 2026
…ards) CI run on PR #750 surfaced a regression in test_memory_json_churn.ts: under all three GC profiles (default, mark-sweep, gen-gc+wb) the test went from PASS at ~120 MB pre-fix to FAIL at 461 MB (limit 250 MB). The churn test allocates ~13 KB per iteration across 5k iterations into a deliberately fragmented arena where every block holds both live and dead objects — so a GC cycle sweeps 91-95% of bytes dead but reclaims *zero* blocks, step-doubles on the productive sweep heuristic, raises the trigger, and the cascade compounds. The original bytes-bump correctly deferred GC indefinitely on that shape; my v0.5.900 fix made GC fire and exposed the fragmentation cascade. Revised the guards to gate on the suppressed window's actual arena growth and split tiny-parse vs medium-or-larger parse handling: (1) `GC_PRE_SUPPRESS_BYTES` — gc_suppress snapshots arena_total so gc_bump_malloc_trigger can compute parse_growth. (2) `GC_TRIGGER_BUMPED` — once-per-cycle flag, applied only when parse_growth >= 1 MB (the json_pipeline_full and json_polyglot shapes). Cleared at the top of gc_collect_inner so all collection entry points re-arm. Tiny parses (< 1 MB, the churn shape) bypass the flag and bump every call, which preserves the pre-fix deferred-GC behavior. (3) Step cap at `GC_THRESHOLD_INITIAL_BYTES` (64 MB) — bound the bump's effective step so post-73a48ced step-doubling can't grant hundreds of MB of headroom per bump. Validation after revision: benchmarks/json_polyglot (lazy-tape default): roundtrip peak RSS: 250 → 223 MB (-11%, was -14% under v1) field-access peak RSS: 407 → 305 MB (-25%, was -39% under v1) scripts/run_memory_stability_tests.sh: 18/18 PASS (was 15/18 PASS, 3/18 FAIL under v1) test_memory_json_churn lands at 209 MB (limit 250) Pipeline-full-style probe: ~135 ms, no mid-iterate GC, 217 MB — the json_pipeline_full optimization is preserved because the 108 MB parse blows past the 1 MB tiny-parse threshold and the once-per-cycle flag still allows the first bump. perry-runtime tests: 250/0/0. Versioning: rebased onto current main (v0.5.902 → v0.5.903) since #743 took v0.5.900 and #749 took v0.5.901/v0.5.902 while this PR was in CI. Updated changelog entry text to match the revised approach.
proggeramlug
added a commit
that referenced
this pull request
May 13, 2026
* fix(gc): #745 — JSON polyglot RSS regression (v0.5.903) `gc_bump_malloc_trigger` was ratcheting the bytes-trigger up on every gc-suppressed parse. For a single 108 MB parse (json_pipeline_full, v0.5.279's original optimization target) that's correct. For a 50-iter loop of `JSON.parse + discard` (json_polyglot's ~5 MB parses) it suppressed GC entirely — the trigger climbed hundreds of MB above the actual live set before allocation could catch up. Two guards in gc.rs: 1. GC_TRIGGER_BUMPED — bump the trigger at most once per GC cycle. Flag is cleared at the top of gc_collect_inner (covers full, minor, manual gc(), and malloc-count trigger paths). 2. GC_PRE_SUPPRESS_BYTES — gc_suppress snapshots arena_total_bytes() so the bump can compute actual parse growth. Skip the bump if growth is below 32 MB. This cleanly separates json_pipeline_full (108 MB parse, bumps) from json_polyglot (~5 MB parse, no bump). Validation: lazy-roundtrip 250 → 215 MB (-14%), lazy-field-access 407 → 246 MB (-39%) at unchanged wall time. GC fires 2× per polyglot run vs 0× pre-fix. perry-runtime tests 250/0/0. * fix(gc): #745 follow-up — preserve test_memory_json_churn (revised guards) CI run on PR #750 surfaced a regression in test_memory_json_churn.ts: under all three GC profiles (default, mark-sweep, gen-gc+wb) the test went from PASS at ~120 MB pre-fix to FAIL at 461 MB (limit 250 MB). The churn test allocates ~13 KB per iteration across 5k iterations into a deliberately fragmented arena where every block holds both live and dead objects — so a GC cycle sweeps 91-95% of bytes dead but reclaims *zero* blocks, step-doubles on the productive sweep heuristic, raises the trigger, and the cascade compounds. The original bytes-bump correctly deferred GC indefinitely on that shape; my v0.5.900 fix made GC fire and exposed the fragmentation cascade. Revised the guards to gate on the suppressed window's actual arena growth and split tiny-parse vs medium-or-larger parse handling: (1) `GC_PRE_SUPPRESS_BYTES` — gc_suppress snapshots arena_total so gc_bump_malloc_trigger can compute parse_growth. (2) `GC_TRIGGER_BUMPED` — once-per-cycle flag, applied only when parse_growth >= 1 MB (the json_pipeline_full and json_polyglot shapes). Cleared at the top of gc_collect_inner so all collection entry points re-arm. Tiny parses (< 1 MB, the churn shape) bypass the flag and bump every call, which preserves the pre-fix deferred-GC behavior. (3) Step cap at `GC_THRESHOLD_INITIAL_BYTES` (64 MB) — bound the bump's effective step so post-73a48ced step-doubling can't grant hundreds of MB of headroom per bump. Validation after revision: benchmarks/json_polyglot (lazy-tape default): roundtrip peak RSS: 250 → 223 MB (-11%, was -14% under v1) field-access peak RSS: 407 → 305 MB (-25%, was -39% under v1) scripts/run_memory_stability_tests.sh: 18/18 PASS (was 15/18 PASS, 3/18 FAIL under v1) test_memory_json_churn lands at 209 MB (limit 250) Pipeline-full-style probe: ~135 ms, no mid-iterate GC, 217 MB — the json_pipeline_full optimization is preserved because the 108 MB parse blows past the 1 MB tiny-parse threshold and the once-per-cycle flag still allows the first bump. perry-runtime tests: 250/0/0. Versioning: rebased onto current main (v0.5.902 → v0.5.903) since #743 took v0.5.900 and #749 took v0.5.901/v0.5.902 while this PR was in CI. Updated changelog entry text to match the revised approach.
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
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.
Summary
Closes #741, closes #742, partial fix for #740. Two commits, version-stacked after #744 (which lands at v0.5.895):
tty.ReadStream/tty.WriteStreamclass exports undefined;stdout.isTTYtypeof is boolean when piped #742 (tty exports/isTTY) + the narrower perry-runtime: Effect framework throwsTypeError: value is not a functionduringParseResult.ts__init—class X extends TaggedError(...)<...>(Effect end-to-end blocker, post-#711) #740 fix (class-expression-as-value lowers to ClassRef, not zero-arg New)TypeError: value is not a functionduringParseResult.ts__init—class X extends TaggedError(...)<...>(Effect end-to-end blocker, post-#711) #740 follow-up: object-literal class-field aliases soconst O = { Inner: class … }; new O.Inner(args)andconst C = O.Inner; new C(args)dispatch to the real classWhat's in #741 (path)
path.dirname("/")→""js_path_dirnamepath.join("/foo/", "/bar/", "baz")→/bar/bazjs_path_joinas concat-then-normalize (was usingPathBuf::join, which haspath.resolvesemantics, notpath.joinsemantics)path.matchesGlobundefinedjs_path_matches_glob(glob→regex via existingregexcrate)path.toNamespacedPaththrew TypeErrorjs_path_to_namespaced_path— POSIX no-opSide effect: changing
js_path_joinsemantics broke the existingpath.resolve(a, b, c)lowering (which chainedPathJoinand was unknowingly relying on itsPath::joinreset-on-absolute). Split into a dedicatedPathResolveJoinHIR variant +js_path_resolve_joinruntime helper.5-line repro from #741 matches Node byte-for-byte.
test-files/test_parity_path.tsdiff drops from 15 lines to 2 (posix.join,win32.join— sub-namespace methods, separate scope).What's in #742 (tty)
typeof tty.ReadStream/tty.WriteStream/tty.isatty→"undefined"js_native_module_property_by_name, whitelist these as callable exports and synthesize aBOUND_METHOD_FUNC_PTRclosure — closures NaN-box with the magic header thatjs_value_typeofreports as"function"process.std{in,out,err}.isTTY→false(typeofboolean) when pipedTAG_UNDEFINEDper Node's presence-test spec.tty.isatty(fd)itself still returns a real boolean5-line repro from #742 matches Node byte-for-byte.
test-files/test_parity_tty.tsparity test now passes byte-for-byte (was 6 lines off).What's in #740 (partial)
Two layered fixes that move multiple shapes from "throws or returns undefined fields" to "constructs correctly":
Expr::New { class_name, args: vec![] }— an empty-arg instance — instead ofExpr::ClassRef(class_name). Soconst C = class { ... }; new C(args)ran the ctor with no args;O.Innerinside{ Inner: class … }held a stillborn instance.FnCtx.local_class_field_aliasespopulated whenStmt::Letseesinit = New { __AnonShape, args }— anyExpr::ClassRefarg becomes a(local_id, field_name) → class_nameentry. Bothnew O.Inner(args)(inNewDynamicCase 4) andconst C = O.Inner; new C(args)(inStmt::Let) consult this. Map propagates throughlet O2 = O.What still doesn't work
The full Effect DoD (#321) needs runtime parent-constructor dispatch:
class ParseError extends TaggedError("ParseError")reaches[3] pe._tag: undefinedbecauselower_newwalks only the staticextends_namechain.RegisterClassParentDynamicregisters the parent at runtime, but no code path consults that registry to run the parent's field initializers. Fix needs each class's constructor emitted as a separately addressable function plus a runtime constructor registry — significantly more architectural work than fit in this session. Documented in changelog.Validation
test_parity_path.ts: diff 2 lines (was 15)test_parity_tty.ts: byte-for-byte match (was 6 lines off)test_gap_*: 26/28 pass — same 2 pre-existing failures as main (known categorical gaps)test_*class*: 20/23 pass — same 3 pre-existing differences as maincargo test -p perry-runtime --lib: 250 passed, 0 failedTest plan
path.dirname("/")returns/path.join("/foo/", "/bar/", "baz")returns/foo/bar/bazpath.matchesGlob("foo.txt", "*.txt")returnstruepath.toNamespacedPath("/foo/bar")returns/foo/bar(no throw)typeof tty.ReadStream/WriteStream/isattyreturns"function"process.stdout.isTTYisundefinedwhen piped (notfalse)const O = { Inner: class … }; new O.Inner(args).fieldreturns the expected valuefunction f() { const C = class…; return C } new f()(args)returns expectedclass X extends Factory()— known still-failing, documented; needs separate architectural work