Skip to content

Optimize FixedArray reverse iteration - #3784

Closed
mizchi wants to merge 2 commits into
moonbitlang:mainfrom
mizchi:perf/fixedarray-reverse-fast
Closed

Optimize FixedArray reverse iteration#3784
mizchi wants to merge 2 commits into
moonbitlang:mainfrom
mizchi:perf/fixedarray-reverse-fast

Conversation

@mizchi

@mizchi mizchi commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Optimize reverse iteration helpers on FixedArray by using unchecked reads in the already bounds-controlled loops.

This covers:

  • FixedArray::rev_each
  • FixedArray::rev_eachi
  • FixedArray::rev_fold
  • FixedArray::rev_foldi

Benchmarks

Command: moon bench -p moonbitlang/core/builtin -f array_locals_bench_test.mbt --target native --release

Benchmark Before After
FixedArray::rev_each n=100000 24.39 us 4.76 us
FixedArray::rev_eachi n=100000 9.51 us 9.34 us
FixedArray::rev_fold n=100000 24.22 us 4.99 us
FixedArray::rev_foldi n=100000 49.48 us 9.75 us

Validation

  • moon fmt
  • moon test builtin --target all
  • moon check builtin --target all --no-render
  • moon info
  • moon bench -p moonbitlang/core/builtin -f array_locals_bench_test.mbt --target native --release

@mizchi
mizchi marked this pull request as ready for review July 3, 2026 08:32
Copilot AI review requested due to automatic review settings July 3, 2026 08:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes FixedArray reverse-iteration helpers in the MoonBit core standard library by switching to unchecked element reads (unsafe_get) inside already bounds-controlled loops to reduce overhead.

Changes:

  • Use unsafe_get inside FixedArray::rev_each, FixedArray::rev_eachi, and FixedArray::rev_fold loops.
  • Refactor FixedArray::rev_foldi to an explicit (read, index, acc) loop and use unsafe_get for the reverse read.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread builtin/fixedarray.mbt
@mizchi
mizchi force-pushed the perf/fixedarray-reverse-fast branch 2 times, most recently from 18df173 to 319a481 Compare July 7, 2026 12:35
@bobzhang

Copy link
Copy Markdown
Contributor

Three of this PR's four optimizations have already landed on main. The remaining piece — rev_foldi's per-iteration len - i - 1 — is now completed in #4117 with fresh benchmarks against current main (native 1.33x, js 1.21x, wasm-gc 1.15x), keeping the range form the sibling functions use (for i in self.length()>..0; index = 0, acc = init), which measures identical to the C-style loop proposed here. Your index-pairing regression test is adopted there with credit. If #4117 lands, this PR can be closed in its favor.

bobzhang added a commit that referenced this pull request Aug 20, 2026
…iteration

Completes PR #3784 (mizchi) on current main, where three of its four
FixedArray reverse-iteration optimizations have already landed. The
remaining delta is rev_foldi, which computed the ascending index as
len - i - 1 on every iteration inside the descending range loop.

Instead of that PR's C-style three-variable loop, the index becomes a
second loop state of the same readable range form:

  for i in self.length()>..0; index = 0, acc = init {
    continue index + 1, f(index, acc, self.unsafe_get(i))
  }

Measured identical to the C-style loop (11.19 vs 11.20 us native) and
faster than main on all three backends (rev_foldi, n=100K, sum fold):
native 14.88 -> 11.19us (1.33x), js 72.4 -> 59.9us (1.21x), wasm-gc
75.5 -> 65.6us (1.15x). Per-element times (~0.1ns) show the fold is
fully inlined and auto-vectorized in this benchmark; the win is the
per-lane subtract dependency of len - i - 1 vectorizing worse than a
counting index. With an opaque, non-inlinable closure the difference
shrinks accordingly.

Also adopts #3784's test pinning the reversed index/element pairing,
which a miscounted index would break.

Codex CLI review (xhigh, first-round sign-off): (index, element)
pairing verified as (0,len-1)..(len-1,0) against the documented
reverse-range and simultaneous-update semantics (f receives the
pre-increment index); the pairing-test trace recomputed to 30120210;
raise-polymorphism and the nobreak return intact; benchmark ratios
recomputed (1.33x/1.21x/1.15x) and microbenchmark-scoped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Codex CLI <codex@openai.com>
@bobzhang bobzhang closed this Aug 20, 2026
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.

3 participants