Skip to content

fix(stdlib): delegate Array reverse/fill/copyWithin/sort to prototype#471

Merged
dowdiness merged 1 commit into
mainfrom
fix/array-fastpath-remove-reverse-fill-copywith-sort
Jun 26, 2026
Merged

fix(stdlib): delegate Array reverse/fill/copyWithin/sort to prototype#471
dowdiness merged 1 commit into
mainfrom
fix/array-fastpath-remove-reverse-fill-copywith-sort

Conversation

@dowdiness

@dowdiness dowdiness commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • Removes the four live fast-path arms for reverse, fill, copyWithin, and sort from get_array_method_with_interp, delegating each to Array.prototype via the normal prototype chain
  • Cleans up the corresponding dead arms in get_array_method (these were unreachable since _with_interp handles them before the _ fallback fires)
  • Deletes the now-dead direct helpers: array_direct_reverse, array_direct_fill, array_direct_copy_within, array_direct_sort_default, and the no-longer-referenced array_mutator_relative_index

Why safe: The removed _with_interp fast paths called the same indexed-property helpers (array_reverse_indexed_properties, array_fill_indexed_properties, etc.) that Array.prototype already uses — the bypass was redundant and skipped length-writable and Proxy trap semantics that route through indexed [[Set]]/[[Delete]].

Net change: −249 lines.

Test plan

  • moon check — clean, 0 warnings
  • moon test — 2249/2249 passed
  • make test262-filter FILTER="reverse" — 64/64 (strict + non-strict)
  • make test262-filter FILTER="fill" — 162/162
  • make test262-filter FILTER="copyWithin" — 150/150
  • make test262-filter FILTER="sort" — 130/144 (14 failures confirmed pre-existing on main)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Updated array method behavior for reverse, fill, copyWithin, and sort to use the shared array prototype path.
    • Improved consistency for array operations when working with indexed elements.
    • Continued support for other array methods such as entries, keys, values, and splice.

The fast-path branches in get_array_method and get_array_method_with_interp
bypassed Array.prototype for reverse, fill, copyWithin, and sort. The
_with_interp paths called the same indexed-property helpers that the prototype
methods already use, so the bypass was redundant and skipped length-writable
and Proxy trap semantics that go through the indexed [[Set]]/[[Delete]] path.

Remove the four live fast-path arms from get_array_method_with_interp, collapse
the corresponding dead arms in get_array_method, and delete the array_direct_*
helpers (array_direct_reverse, array_direct_fill, array_direct_copy_within,
array_direct_sort_default) and the now-unused array_mutator_relative_index
helper. All 2249 unit tests pass; test262 filter rates are unchanged:
reverse 64/64, fill 162/162, copyWithin 150/150, sort 130/144 (14 pre-existing).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6eb28163-e0cf-4475-92ce-3e6e237cf30a

📥 Commits

Reviewing files that changed from the base of the PR and between eced381 and 6875942.

📒 Files selected for processing (2)
  • interpreter/stdlib/array_indexed_mutators.mbt
  • interpreter/stdlib/builtins_array.mbt
💤 Files with no reviewable changes (1)
  • interpreter/stdlib/array_indexed_mutators.mbt

📝 Walkthrough

Walkthrough

Array builtin dispatch now delegates reverse, fill, copyWithin, and sort through prototype handling in both method lookup paths, and the direct ArrayData helper implementations for those operations are removed.

Changes

Array method delegation updates

Layer / File(s) Summary
Builtin dispatch delegates mutators
interpreter/stdlib/builtins_array.mbt
get_array_method and get_array_method_with_interp switch reverse, fill, copyWithin, and sort to delegation-only handling.
Direct ArrayData helpers removed
interpreter/stdlib/array_indexed_mutators.mbt
The relative-index helper and the direct reverse, fill, copyWithin, and default sort ArrayData helpers are deleted.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

(\/)
(•
•) I hopped along the array trail today,
/ >🍃 where reverse and fill now delegate away.
CopyWithin and sort took a gentler lane,
My little rabbit nose says: neat and plain!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: delegating Array reverse/fill/copyWithin/sort to the prototype.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/array-fastpath-remove-reverse-fill-copywith-sort

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Benchmark Results

Run: https://github.com/dowdiness/js_engine/actions/runs/28223365312

startup/tiny_program is the PR #153 / issue #141 guardrail for built-in realm-stamping startup cost.

Stage summary

stage benchmarks total mean slowest benchmark slowest mean noisy rows
startup 3 2.569 ms startup/tiny_program 1.299 ms 0
frontend 7 0.931 ms pipeline/parse_heavy 0.514 ms 2
execution 26 15293.423 ms exec/fibonacci_30 13822.939 ms 2

Focused bytecode base-vs-head comparison

Base-vs-head deltas are reporting-only. Negative delta and PR/base < 1.00x mean the PR is faster; interpret high-CV or noisy rows cautiously.

benchmark stage base mean PR mean delta PR/base base CV PR CV noisy
baseline/bytecode/closure_factory execution 13.844 ms 14.046 ms +1.5% 1.01x 9.7% 6.2% no
pipeline/bytecode/evaluate execution 9.390 ms 8.695 ms -7.4% 0.93x 1.9% 2.5% no
isolate/bytecode/call_frame execution 7.701 ms 7.333 ms -4.8% 0.95x 4.9% 0.5% no
isolate/bytecode/runtime_helpers execution 10.681 ms 10.940 ms +2.4% 1.02x 0.5% 0.4% no
isolate/bytecode/local_access execution 36.959 ms 39.236 ms +6.2% 1.06x 0.9% 2.9% no
isolate/bytecode/env_access execution 36.848 ms 38.756 ms +5.2% 1.05x 1.7% 2.9% no
isolate/bytecode/captured_access execution 37.313 ms 36.331 ms -2.6% 0.97x 1.3% 1.8% no
isolate/bytecode/dispatch_stack execution 23.160 ms 23.690 ms +2.3% 1.02x 0.6% 1.6% no

Base-vs-head comparison

benchmark stage base mean PR mean delta PR/base base CV PR CV noisy
startup/tiny_program startup 1.217 ms 1.299 ms +6.8% 1.07x 5.9% 3.5% no
lexer/small frontend 0.034 ms 0.035 ms +1.5% 1.02x 24.2% 25.8% base, PR
lexer/large frontend 0.299 ms 0.300 ms +0.3% 1.00x 0.5% 0.7% no
exec/fibonacci_30 execution 13206.599 ms 13822.939 ms +4.7% 1.05x 0.5% 0.4% no
exec/property_chain execution 17.600 ms 15.889 ms -9.7% 0.90x 31.7% 10.1% base
startup/phase/parse_tiny frontend 0.002 ms 0.002 ms -2.5% 0.97x 0.5% 0.6% no
startup/phase/new_interpreter startup 1.234 ms 1.270 ms +3.0% 1.03x 10.3% 10.6% no
startup/phase/execute_preparsed_tiny execution 0.001 ms 0.001 ms -8.7% 0.91x 0.7% 1.2% no
startup/phase/event_loop_drain_empty startup 0.000 ms 0.000 ms +3.5% 1.03x 0.8% 1.0% no
startup/phase/result_stringify_output execution 0.000 ms 0.000 ms +0.2% 1.00x 0.6% 1.6% no
exec/array_map_filter execution 21.181 ms 20.848 ms -1.6% 0.98x 20.2% 17.0% base, PR
exec/closure_factory execution 29.697 ms 31.552 ms +6.2% 1.06x 6.7% 8.5% no
baseline/closure_legacy/closure_factory execution 27.697 ms 28.258 ms +2.0% 1.02x 8.0% 9.0% no
baseline/bytecode/closure_factory execution 13.844 ms 14.046 ms +1.5% 1.01x 9.7% 6.2% no
isolate/bytecode/dispatch_stack execution 23.160 ms 23.690 ms +2.3% 1.02x 0.6% 1.6% no
isolate/bytecode/local_access execution 36.959 ms 39.236 ms +6.2% 1.06x 0.9% 2.9% no
isolate/bytecode/env_access execution 36.848 ms 38.756 ms +5.2% 1.05x 1.7% 2.9% no
isolate/bytecode/captured_access execution 37.313 ms 36.331 ms -2.6% 0.97x 1.3% 1.8% no
isolate/bytecode/call_frame execution 7.701 ms 7.333 ms -4.8% 0.95x 4.9% 0.5% no
isolate/bytecode/runtime_helpers execution 10.681 ms 10.940 ms +2.4% 1.02x 0.5% 0.4% no
isolate/bytecode/property_get execution 42.758 ms 44.649 ms +4.4% 1.04x 2.8% 1.7% no
isolate/bytecode/property_set execution 40.255 ms 40.858 ms +1.5% 1.01x 1.2% 0.8% no
isolate/bytecode/method_call execution 8.298 ms 8.374 ms +0.9% 1.01x 0.9% 0.4% no
isolate/bytecode/object_literal execution 13.914 ms 12.789 ms -8.1% 0.92x 1.1% 0.6% no
isolate/bytecode/array_literal execution 14.948 ms 14.645 ms -2.0% 0.98x 0.9% 1.4% no
exec/for_of execution 6.338 ms 5.921 ms -6.6% 0.93x 9.1% 6.2% no
exec/arithmetic_loop execution 1018.451 ms 1007.042 ms -1.1% 0.99x 0.6% 0.6% no
exec/object_construction execution 7.943 ms 7.164 ms -9.8% 0.90x 5.2% 6.6% no
exec/string_ops execution 2.134 ms 1.933 ms -9.4% 0.91x 21.4% 20.5% base, PR
pipeline/exec/lex frontend 0.031 ms 0.031 ms +0.7% 1.01x 0.9% 0.8% no
pipeline/exec/parse frontend 0.027 ms 0.027 ms +0.0% 1.00x 2.9% 2.9% no
pipeline/exec/evaluate execution 28.077 ms 26.555 ms -5.4% 0.95x 7.9% 11.5% no
pipeline/closure_legacy/evaluate execution 26.055 ms 24.981 ms -4.1% 0.96x 4.0% 4.4% no
pipeline/bytecode/compile frontend 0.022 ms 0.022 ms +1.4% 1.01x 37.6% 32.7% base, PR
pipeline/bytecode/evaluate execution 9.390 ms 8.695 ms -7.4% 0.93x 1.9% 2.5% no
pipeline/parse_heavy frontend 0.517 ms 0.514 ms -0.6% 0.99x 4.1% 5.4% no

Mean-time chart (log scale)

benchmark stage mean chart
startup/tiny_program startup 1.299 ms ##
lexer/small frontend 0.035 ms ⚠ #
lexer/large frontend 0.300 ms #
exec/fibonacci_30 execution 13822.939 ms ##############################
exec/property_chain execution 15.889 ms ########
startup/phase/parse_tiny frontend 0.002 ms #
startup/phase/new_interpreter startup 1.270 ms ##
startup/phase/execute_preparsed_tiny execution 0.001 ms #
startup/phase/event_loop_drain_empty startup 0.000 ms #
startup/phase/result_stringify_output execution 0.000 ms #
exec/array_map_filter execution 20.848 ms ⚠ #########
exec/closure_factory execution 31.552 ms ##########
baseline/closure_legacy/closure_factory execution 28.258 ms ##########
baseline/bytecode/closure_factory execution 14.046 ms ########
isolate/bytecode/dispatch_stack execution 23.690 ms ##########
isolate/bytecode/local_access execution 39.236 ms ###########
isolate/bytecode/env_access execution 38.756 ms ###########
isolate/bytecode/captured_access execution 36.331 ms ###########
isolate/bytecode/call_frame execution 7.333 ms ######
isolate/bytecode/runtime_helpers execution 10.940 ms #######
isolate/bytecode/property_get execution 44.649 ms ############
isolate/bytecode/property_set execution 40.858 ms ###########
isolate/bytecode/method_call execution 8.374 ms #######
isolate/bytecode/object_literal execution 12.789 ms ########
isolate/bytecode/array_literal execution 14.645 ms ########
exec/for_of execution 5.921 ms ######
exec/arithmetic_loop execution 1007.042 ms #####################
exec/object_construction execution 7.164 ms ######
exec/string_ops execution 1.933 ms ⚠ ###
pipeline/exec/lex frontend 0.031 ms #
pipeline/exec/parse frontend 0.027 ms #
pipeline/exec/evaluate execution 26.555 ms ##########
pipeline/closure_legacy/evaluate execution 24.981 ms ##########
pipeline/bytecode/compile frontend 0.022 ms ⚠ #
pipeline/bytecode/evaluate execution 8.695 ms #######
pipeline/parse_heavy frontend 0.514 ms #

Closure-conversion comparison

  • unavailable

@dowdiness dowdiness merged commit d530525 into main Jun 26, 2026
15 checks passed
@dowdiness dowdiness deleted the fix/array-fastpath-remove-reverse-fill-copywith-sort branch June 26, 2026 07:41
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.

1 participant