perf: improve @list.from_iter use one pass

[illusory0x0]

No description provided.

[peter-jerry-ye-code-review]

New implementation avoids unnecessary list reversal but uses more memory with two pointers

Category
Performance
Code Snippet
Original: iter.fold(init=Empty, fn(acc, e) { More(e, tail=acc) }).reverse_inplace()
New: Uses mut res and ptr
Recommendation
The new implementation is actually better for performance since it avoids the O(n) reversal operation. Keep the new implementation.
Reasoning
The original implementation had to reverse the entire list after building it, which required an extra pass through all elements. The new implementation builds the list in the correct order directly.

The abort case seems unreachable but could be more explicit

Category
Correctness
Code Snippet
(, ) => abort("unreachable")
Recommendation
Consider removing the catch-all pattern since the first two patterns should cover all possible cases, or document why it's needed
Reasoning
If res is More(), ptr should always be More() in this implementation. The catch-all pattern suggests there might be other cases, which could be confusing for maintainers.

Complex pointer manipulation logic could benefit from additional comments

Category
Maintainability
Code Snippet
match (res, ptr) {...}
Recommendation
Add comments explaining the pointer manipulation logic:

// Initialize list if empty
(Empty, _) => {...}
// Append new element to tail
(More(_), More(_) as ptr_) => {...}

Reasoning
The pointer manipulation pattern is a common source of bugs and confusion. Clear documentation would help future maintainers understand the implementation.

[coveralls]

Pull Request Test Coverage Report for Build 7002

Details

• 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.007%) to 92.491%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
list/list.mbt	4	5	80.0%

Totals
Change from base Build 7000:	-0.007%
Covered Lines:	8745
Relevant Lines:	9455

---

💛 - Coveralls

[peter-jerry-ye]

How's the performance comparison?

[illusory0x0]

one pass implementation is a litter faster than original one.

bench result

illu@illusory0x0 ~/d/b/moon-bench (master)> moon bench --target wasm,native
bench username/hello/top.mbt::0
name time (mean ± σ)         range (min … max) 
v1   45.83 µs ±   1.32 µs    45.11 µs …  49.47 µs  in 10 ×   2196 runs
v2   49.36 µs ±   0.28 µs    49.06 µs …  49.95 µs  in 10 ×   2037 runs
Total tests: 1, passed: 1, failed: 0. [wasm]
bench username/hello/top.mbt::0
name time (mean ± σ)         range (min … max) 
v1   27.75 µs ±   0.17 µs    27.60 µs …  28.12 µs  in 10 ×   3636 runs
v2   28.79 µs ±   0.49 µs    28.23 µs …  29.56 µs  in 10 ×   3402 runs
Total tests: 1, passed: 1, failed: 0. [native]

Example

pub enum T[A] {
  Empty
  More(A, mut tail~ : T[A])
} derive(Eq)


pub fn from_iter_v1[A](iter : Iter[A]) -> T[A] {
  let mut res = Empty
  let mut ptr = Empty
  for x in iter {
    match (res, ptr) {
      (Empty, _) => {
        res = More(x, tail=Empty)
        ptr = res
      }
      (More(_), More(_) as ptr_) => {
        ptr_.tail = More(x, tail=Empty)
        ptr = ptr_.tail
      }
      (_, _) => abort("unreachable")
    }
  }
  res
}


pub fn from_iter_v2[A](iter : Iter[A]) -> T[A] {
  iter.fold(init=Empty, fn(acc, e) { More(e, tail=acc) }).reverse_inplace()

}

fn reverse_inplace[A](self : T[A]) -> T[A] {
  match self {
    Empty | More(_, tail=Empty) => self
    More(head, tail~) =>
      loop More(head, tail=Empty), tail {
        result, Empty => result
        result, More(_, tail=xs) as new_result => {
          new_result.tail = result
          continue new_result, xs
        }
      }
  }
}


test (b : @bench.T) {
  let xs = Int::until(0,1000)

  b.bench(name="v1",fn() { b.keep(from_iter_v1(xs)) })
  b.bench(name="v2",fn() { b.keep(from_iter_v2(xs)) })
}

[peter-jerry-ye]

It looks drastically faster on wasm-gc. Seems to be a reasonable change.

[illusory0x0]

It looks drastically faster on wasm-gc. Seems to be a reasonable change.

benchmark result is unstable in js backend and wasm-gc backend due to GC.

moonbitlang/moon#813

[illusory0x0]

@peter-jerry-ye Should we close this PR since this change is a weak performance boost?