-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(runtime): reduce arguments object construction overhead #10081
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| function-arguments.ts text eol=lf |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| # Arguments construction benchmark (#10063) | ||
|
|
||
| `function-arguments.ts` is the unchanged standalone workload from | ||
| [issue #10063](https://github.com/PerryTS/perry/issues/10063). Its SHA-256 is | ||
| `f1ae42e13cdf29dec7829aa402c3daf22f093f4a17911dabbc0743c4e105ba73`. | ||
| The local `package.json` preserves the original standalone file's CommonJS | ||
| context, including sloppy mapped arguments, despite the repository's root | ||
| `"type": "module"` setting. | ||
|
|
||
| ## Reproduce | ||
|
|
||
| Use Node v26.5.1 and the same Rust/LLVM toolchain for both Perry builds. In | ||
| separate clean worktrees for the baseline and patched revisions, build the CLI | ||
| and both runtime archives together: | ||
|
|
||
| ```sh | ||
| cargo build --release --locked -p perry -p perry-runtime-static -p perry-stdlib-static | ||
| ``` | ||
|
|
||
| For each build, point `PERRY_RUNTIME_DIR` at that worktree's `target/release` and | ||
| compile this same source with `--no-auto-optimize` into separate executables: | ||
|
|
||
| ```sh | ||
| PERRY_RUNTIME_DIR=/absolute/baseline/target/release \ | ||
| /absolute/baseline/target/release/perry compile \ | ||
| /absolute/patched/benchmarks/issue-10063/function-arguments.ts \ | ||
| --no-auto-optimize -o /absolute/results/before | ||
| PERRY_RUNTIME_DIR=/absolute/patched/target/release \ | ||
| /absolute/patched/target/release/perry compile \ | ||
| /absolute/patched/benchmarks/issue-10063/function-arguments.ts \ | ||
| --no-auto-optimize -o /absolute/results/after | ||
| python benchmarks/issue-10063/measure.py \ | ||
| --source benchmarks/issue-10063/function-arguments.ts \ | ||
| --before /absolute/results/before --after /absolute/results/after \ | ||
| --output /absolute/results/comparison.json --rounds 3 | ||
| ``` | ||
|
|
||
| On Windows, use `perry.exe`, `.exe` output paths, and PowerShell's | ||
| `$env:PERRY_RUNTIME_DIR = 'C:\absolute\worktree\target\release'` syntax. | ||
| Ensure LLVM's binaries are on `PATH`. Do not set a different GC backend for one | ||
| build or run other builds or benchmarks during the comparison. | ||
|
|
||
| The driver serializes every process, alternates the two native builds' order, | ||
| enforces a 60-second whole-process timeout, and checks all five checksums. The | ||
| workload retains its original warmup (at least 200 ms and five runs), seven | ||
| samples, and at least 20 ms per sample. Setup, checksum validation, and process | ||
| startup are outside the reported `ms_per_run`; `wall_seconds` includes them. | ||
|
|
||
| ## Diagnosis | ||
|
|
||
| A Windows program-counter sample of the baseline's 10,000-element workload | ||
| recorded 1,298 samples. Among the 60 most frequent symbols, shape bookkeeping | ||
| accounted for 445 self samples, GC for 252, and descriptor helpers for 85. | ||
| Symbols were resolved from the native linker's map; these are self samples, | ||
| not inclusive stack costs. The hot path repeatedly appended argument keys, | ||
| registered redundant all-true indexed descriptors, and changed semantic shapes. | ||
|
|
||
| The fix constructs ordinary arguments objects in bulk. A bounded, GC-traced | ||
| cache shares immutable key arrays for arities 0 through 64. Each call still gets | ||
| its own values, identity, descriptors, and mapped boxes; key mutation uses the | ||
| existing copy-on-write mechanism. Indexed descriptors use their existing | ||
| all-true defaults, and the two initial sloppy descriptors share one invalidation | ||
| and shape transition. | ||
|
|
||
| ## Measurements | ||
|
|
||
| Measured on 2026-09-11 on Windows x86_64, AMD Ryzen 5 7640HS (12 logical | ||
| processors), Node v26.5.1, LLVM 22.1.8, and the Rust nightly 2026-08-20 toolchain. | ||
| Both Perry builds report 0.5.1532 and use the same release profile (optimization | ||
| level 3, thin LTO, one codegen unit). The baseline is | ||
| `603b074ace01464bc66fc07cc8d532f26ccf5a0f`. Timing used the default native | ||
| statepoint backend and no concurrent builds or test suites. | ||
|
|
||
| Each cell is the median of three processes' reported medians, in milliseconds | ||
| per workload run. The million-call baseline produced no result within the | ||
| 60-second process budget in any round; its speedup cannot be quantified here. | ||
|
|
||
| | Calls | Node | Before | After | Speedup | | ||
| | ---: | ---: | ---: | ---: | ---: | | ||
| | 100 | 0.002337 | 0.7342 | 0.2108 | 3.48x | | ||
| | 1,000 | 0.02261 | 7.413 | 2.228 | 3.33x | | ||
| | 10,000 | 0.2254 | 75.15 | 21.02 | 3.57x | | ||
| | 100,000 | 1.940 | 1,697 | 302.5 | 5.61x | | ||
| | 1,000,000 | 18.54 | timeout (3/3) | 3,317 | n/a | | ||
|
|
||
| All patched runs and all completed baseline runs matched Node's checksums. | ||
| The patched million-call processes took 40.109, 40.984, and 42.046 seconds in | ||
| total, including warmup and all seven samples. Their reported medians ranged | ||
| from 3,311.6 to 3,507.8 ms per workload run. Perry still has considerable overhead | ||
| relative to Node; these results establish the improvement on this Windows host, | ||
| not a direct comparison with the issue's original macOS measurements. | ||
|
|
||
| See [results-windows.json](results-windows.json) for every observation, timeouts, | ||
| toolchain details, and artifact/source hashes. | ||
|
|
||
| ## Semantic validation | ||
|
|
||
| `cargo test --release --locked --lib -p perry-runtime arguments -- --test-threads=1` | ||
| passes all 10 selected tests, including the four new construction and moving-GC | ||
| regressions. The full runtime library suite passes 3,439 tests, ignores four, | ||
| and fails `emergency_full_trace_is_excluded_from_ordinary_pause_stats` on this | ||
| Windows host. That unchanged telemetry test expects allocator trimming to be | ||
| unsupported, whereas the unchanged mimalloc path reports that it executed; it | ||
| also fails in isolation. | ||
|
|
||
| With Test262 pinned at `4249661388e5d3f92a85186213da140a6481490f`, both builds | ||
| pass 246 of 261 `language/arguments-object` cases. All 15 remaining failure | ||
| paths, buckets, and reasons match exactly. Both builds also pass eight of the | ||
| nine repository fixtures selected by `run_parity_tests.sh --filter arguments`; | ||
| `test_issue_3580_arguments_object_semantics` produces the same existing mismatch. | ||
|
|
||
| For those conformance runs, set `PYTHONUTF8=1` so Python writes valid UTF-8 | ||
| harness files on Windows, and `PERRY_RS4GC=0` for both builds. The default native | ||
| statepoint backend currently rejects the Windows exception-handling code used | ||
| by most of these tests. The timings above use the default backend, and the new | ||
| Rust GC tests independently assert actual evacuation of cached keys, escaping | ||
| arguments objects, and indexed heap references. | ||
|
|
||
| Runtime formatting, test registration, file-size, GC store, address-class, | ||
| root-holder, rekey, and raw-handle audits pass. `pre-tag-check.sh --quick` reports | ||
| two remaining checkout/host limitations: workspace formatting exceeds Windows' | ||
| command-length limit, and the public benchmark evidence is stale (also observed | ||
| on the baseline). The workspace version, CLAUDE.md, and CHANGELOG.md are unchanged. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| // @runtime {"name": "function-arguments", "category": "functions-async", "verification": "checksum", "sources": [{"file": "crates/perry-runtime/src/object/arguments.rs", "function": "js_arguments_object_alloc"}], "hypothesis": "Arguments objects install indexed properties and descriptors individually.", "notes": "", "asynchronous": false, "output_stderr": false, "fresh_input": false} | ||
| // Standalone file. Shared helpers/driver are inlined by common.py. | ||
|
|
||
| let seed = 0x12345678; | ||
| function rnd(): number { | ||
| seed ^= seed << 13; seed ^= seed >>> 17; seed ^= seed << 5; | ||
| return (seed >>> 0) / 4294967296; | ||
| } | ||
| function numbers(n: number): number[] { | ||
| const a: number[] = []; | ||
| for (let i = 0; i < n; i++) a.push(Math.floor(rnd() * 1000000)); | ||
| return a; | ||
| } | ||
| function hashArray(a: number[]): number { | ||
| let h = a.length; | ||
| for (let i = 0; i < a.length; i++) h = (h * 31 + a[i]) % 1000000007; | ||
| return h; | ||
| } | ||
| // Bounded checksum work avoids making string slicing/indexing part of every | ||
| // string benchmark's asymptotic cost. The workload itself consumes its result. | ||
| function hashString(s: string): number { | ||
| let h = s.length; | ||
| const step = Math.max(1, Math.floor(s.length / 32)); | ||
| for (let i = 0; i < s.length; i += step) h = (h * 31 + s.charCodeAt(i)) % 1000000007; | ||
| return h; | ||
| } | ||
|
|
||
| function sumArgs(a: number, b: number, c: number): number { | ||
| let h = 0; | ||
| for (let j = 0; j < arguments.length; j++) h += arguments[j] * (j + 1); | ||
| return h; | ||
| } | ||
| function setup(n: number): number[] { return numbers(n); } | ||
| function run(input: number[]): number { | ||
| let h = 0; | ||
| for (let i = 0; i < input.length; i++) h = (h + sumArgs(input[i], i % 7, i % 11)) % 1000000007; | ||
| return h; | ||
| } | ||
|
|
||
| // Size is the final argument: both native Perry and Node expose it reliably. | ||
| const n = Number(process.argv[process.argv.length - 1]); | ||
| if (!(n > 0)) throw new Error("Expected a positive size argument"); | ||
| function benchmarkMain(): void { | ||
| seed = 0x12345678; | ||
| const preparedInput = setup(n); | ||
| let checksum = 0; | ||
| let seen = false; | ||
| let warmMs = 0; | ||
| let warmRuns = 0; | ||
| while (warmMs < 200 || warmRuns < 5) { | ||
| seed = 0x12345678; | ||
| const input = preparedInput; | ||
| const start = performance.now(); | ||
| const value = run(input); | ||
| const elapsed = performance.now() - start; | ||
| if (!(elapsed >= 0)) throw new Error("Invalid monotonic timer"); | ||
| warmMs += elapsed; | ||
| warmRuns++; | ||
| if (seen && value !== checksum) throw new Error("CORRECTNESS: unstable checksum during warmup"); | ||
| checksum = value; | ||
| seen = true; | ||
| } | ||
| const samples: number[] = []; | ||
| let runs = 0; | ||
| for (let sample = 0; sample < 7; sample++) { | ||
| let elapsed = 0; | ||
| let count = 0; | ||
| // Mutable workloads prepare fresh input BEFORE each timer; immutable | ||
| // workloads reuse setup. Neither preparation nor validation is measured. | ||
| while (elapsed < 20) { | ||
| seed = 0x12345678; | ||
| const input = preparedInput; | ||
| const start = performance.now(); | ||
| const value = run(input); | ||
| const duration = performance.now() - start; | ||
| if (!(duration >= 0)) throw new Error("Invalid monotonic timer"); | ||
| elapsed += duration; | ||
| count++; | ||
| if (value !== checksum) throw new Error("CORRECTNESS: unstable checksum during sampling"); | ||
| } | ||
| samples.push(elapsed / count); | ||
| runs += count; | ||
| } | ||
| // Do not depend on Array.sort to compute the median of a sort benchmark. | ||
| for (let i = 1; i < samples.length; i++) { | ||
| const v = samples[i]; | ||
| let j = i - 1; | ||
| while (j >= 0 && samples[j] > v) { samples[j + 1] = samples[j]; j--; } | ||
| samples[j + 1] = v; | ||
| } | ||
| console.log(JSON.stringify({name: "function-arguments", category: "functions-async", n, | ||
| ms_per_run: samples[3], runs, checksum})); | ||
| } | ||
| benchmarkMain(); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| """Compare builds of the unchanged #10063 workload, with serialized 60s runs.""" | ||
|
|
||
| import argparse | ||
| import hashlib | ||
| import json | ||
| import os | ||
| from pathlib import Path | ||
| import subprocess | ||
| import time | ||
|
|
||
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument("--source", type=Path, required=True) | ||
| parser.add_argument("--before", type=Path, required=True) | ||
| parser.add_argument("--after", type=Path, required=True) | ||
| parser.add_argument("--output", type=Path, required=True) | ||
| parser.add_argument("--rounds", type=int, default=3) | ||
| args = parser.parse_args() | ||
| node = subprocess.check_output(["node", "--version"], text=True).strip() | ||
| if node != "v26.5.1": | ||
| parser.error(f"use the issue's Node v26.5.1 oracle, found {node}") | ||
| if args.rounds < 1: | ||
| parser.error("--rounds must be positive") | ||
| source = args.source.resolve() | ||
| expected = {100: 53207531, 1000: 509027806, 10000: 6382792, | ||
| 100000: 66481643, 1000000: 475838285} | ||
| commands = {"node": ["node", str(source)], | ||
| "before": [str(args.before.resolve())], | ||
| "after": [str(args.after.resolve())]} | ||
| report = {"node": node, "source_sha256": hashlib.sha256(source.read_bytes()).hexdigest(), | ||
| "timeout_seconds": 60, "rows": []} | ||
| env = {**os.environ, "TZ": "UTC", "LC_ALL": "en_US.UTF-8"} | ||
| failed = False | ||
| for repeat in range(args.rounds): | ||
| # Alternate the native order to reduce order/thermal bias. Never run | ||
| # two benchmark processes concurrently on the measurement host. | ||
| engines = ["node", "before", "after"] if repeat % 2 == 0 else ["node", "after", "before"] | ||
| for size, checksum in expected.items(): | ||
| for engine in engines: | ||
| row = {"repeat": repeat, "engine": engine, "n": size} | ||
| started = time.monotonic() | ||
| try: | ||
| process = subprocess.run(commands[engine] + [str(size)], env=env, | ||
| capture_output=True, text=True, timeout=60) | ||
| row["exit_code"] = process.returncode | ||
| if process.returncode: | ||
| row.update(status="error", stdout=process.stdout, stderr=process.stderr) | ||
| failed = True | ||
| else: | ||
| result = json.loads(process.stdout) | ||
| row.update(result) | ||
| row["status"] = "ok" if result["checksum"] == checksum else "checksum-mismatch" | ||
| failed |= row["status"] != "ok" | ||
| except subprocess.TimeoutExpired: | ||
| row["status"] = "timeout" | ||
| failed |= engine != "before" | ||
| row["wall_seconds"] = time.monotonic() - started | ||
| report["rows"].append(row) | ||
| args.output.write_text(json.dumps(report, indent=2) + "\n", encoding="utf-8") | ||
| print(json.dumps(row), flush=True) | ||
| return int(failed) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main()) | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| {"type":"commonjs"} |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the benchmark source hash before running the measurements.
Line 31 records the hash but does not enforce it. A modified source can retain the expected checksums while removing
argumentsconstruction work. The report would then not support the unchanged-workload claim.Proposed fix
🤖 Prompt for AI Agents