Improve benchmark accuracy - #31
Conversation
📝 WalkthroughWalkthroughRefactored two benchmark modules to a versioned BenchV1 API with a formal Schema and an indexed run(ri, ci) dispatcher; added public init() returning BenchV1. Also updated package metadata and bumped a dev dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant BenchV1 as BenchV1 (init)
participant Dispatcher as run(ri,ci)
participant VectorSort as Vector.sort
participant BufferSort as Buffer.sort
participant ArraySort as Array.sort
Caller->>BenchV1: init() -> BenchV1(schema, run)
Caller->>BenchV1: runCell(ri, ci)
BenchV1->>Dispatcher: run(ri, ci)
alt ri selects vector path
Dispatcher->>VectorSort: sort(vectorInput[ri][ci])
VectorSort-->>Dispatcher: sorted
else ri selects buffer path
Dispatcher->>BufferSort: sort(bufferInput[ri][ci])
BufferSort-->>Dispatcher: sorted
else ri selects array path
Dispatcher->>ArraySort: sort(arrayInput[ri][ci])
ArraySort-->>Dispatcher: sorted
end
Dispatcher-->>Caller: done
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bench/vector-sort.bench.mo`:
- Around line 72-90: The benchmark mutates shared inputs because run calls
Vector.sort and buf.sort on precomputed vectorInput and bufferInput; change run
to operate on clones so each iteration sorts fresh data: inside func run(ri, ci)
create local clones of vectorInput[ri/3][ci] and bufferInput[ri/3][ci] (e.g.
convert to an intermediate Array and then back or use the library's
copy/fromArray/toArray helpers) and call Vector.sort and buf.sort on those
clones; similarly clone arrayInput before calling Array.sort (or use a
non-mutating sort helper) so vectorInput, bufferInput, and arrayInput remain
unchanged across runs.
🧹 Nitpick comments (1)
bench/vector-sort.bench.mo (1)
10-29: Consider extracting shared Schema and BenchV1 definitions.The
Schematype andBenchV1class are duplicated betweenvector.bench.moandvector-sort.bench.mo. If the bench framework allows, consider extracting these to a shared module to reduce duplication.
| let bufferInput = Array.map(arrayInput, func x = Array.map(x, func y = Buffer.fromArray<Nat>(y))); | ||
| let vectorInput = Array.map(arrayInput, func x = Array.map(x, func y = Vector.fromArray<Nat>(y))); | ||
|
|
||
| routines[ci * rows.size() + ri](); | ||
| } | ||
| ); | ||
| func run(ri : Nat, ci : Nat) { | ||
| switch (ri % 3) { | ||
| case (0) { | ||
| let vec = vectorInput[ri / 3][ci]; | ||
| Vector.sort(vec, Nat.compare); | ||
| }; | ||
| case (1) { | ||
| let buf = bufferInput[ri / 3][ci]; | ||
| buf.sort(Nat.compare); | ||
| }; | ||
| case (2) { | ||
| let arr = arrayInput[ri / 3][ci]; | ||
| ignore Array.sort<Nat>(arr, Nat.compare); | ||
| }; | ||
| case (_) Prim.trap("Can never happen"); | ||
| }; |
There was a problem hiding this comment.
Potential issue: In-place sorting mutates shared input data.
vectorInput and bufferInput are created once during init(), but Vector.sort and buf.sort mutate them in-place. After the first benchmark run, these collections will already be sorted, so subsequent iterations (if the framework runs multiple) will measure sorting already-sorted data rather than the original input.
If accurate benchmarking of sorting performance is the goal, consider one of:
- Clone the input before sorting each time
- Regenerate input for each benchmark iteration
- Document that this measures single-run performance only
🤖 Prompt for AI Agents
In `@bench/vector-sort.bench.mo` around lines 72 - 90, The benchmark mutates
shared inputs because run calls Vector.sort and buf.sort on precomputed
vectorInput and bufferInput; change run to operate on clones so each iteration
sorts fresh data: inside func run(ri, ci) create local clones of
vectorInput[ri/3][ci] and bufferInput[ri/3][ci] (e.g. convert to an intermediate
Array and then back or use the library's copy/fromArray/toArray helpers) and
call Vector.sort and buf.sort on those clones; similarly clone arrayInput before
calling Array.sort (or use a non-mutating sort helper) so vectorInput,
bufferInput, and arrayInput remain unchanged across runs.
1 participant
Summary by CodeRabbit
Refactor
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.