Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions bench/bench.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

///|
fn iter_n_microseconds(inner : () -> Unit, k : Int) -> Double {
fn iter_n_nanoseconds(inner : () -> Unit, k : Int) -> Double {
let ts = monotonic_clock_start()
for _ in 0..<k {
inner()
Expand All @@ -25,12 +25,12 @@ fn iter_n_microseconds(inner : () -> Unit, k : Int) -> Double {
///|
fn iter_count(name? : String, inner : () -> Unit, count : UInt) -> Summary {
let count = count.land(0x7FFFFFFF).reinterpret_as_int()
let threshold = 100000.0 // 100 ms
let threshold = 100000000.0 // 100 ms
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Native backend monotonic_clock_end not updated for microsecond-to-nanosecond conversion

The PR converts benchmark timing from microseconds to nanoseconds for JS and WASM backends, but the native/llvm backend (bench/monotonic_clock_native.mbt:23) was not updated. The moonbit_monotonic_clock_stop C extern presumably still returns microseconds (matching the old contract that JS and WASM also returned microseconds). Since bench/bench.mbt:28 now sets threshold = 100000000.0 (100ms in nanoseconds) but native's clock still returns microseconds, the calibration loop in iter_count will interpret μs values as ns — making the threshold effectively 100 seconds instead of 100ms. This causes the warm-up phase to run ~1000× longer than intended on native/llvm targets, and all reported timing statistics will be in microseconds while the code expects nanoseconds.

Prompt for agents
The PR updates the JS backend (monotonic_clock_js.mbt, multiplier 1000.0 -> 1000000.0) and WASM backend (monotonic_clock_wasm.mbt, multiplier 1000000.0 -> 1000000000.0) to return nanoseconds instead of microseconds, but the native/llvm backend in bench/monotonic_clock_native.mbt was not touched. The extern C function moonbit_monotonic_clock_stop presumably still returns microseconds. Either the MoonBit wrapper in monotonic_clock_native.mbt needs to multiply the result by 1000.0 to convert from microseconds to nanoseconds, or the C runtime function itself needs to be updated. Without this fix, benchmarks on native/llvm targets will have the calibration threshold off by 1000x (interpreted as 100 seconds instead of 100ms) and all timing values will be in the wrong unit.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

let target_batch_size = for trial = 1 {
let single_us = iter_n_microseconds(inner, trial) / trial.to_double()
let single_ns = iter_n_nanoseconds(inner, trial) / trial.to_double()
let target_batch_size = threshold /
(if single_us < 1.0 { 1.0 } else { single_us })
if trial.to_double() * single_us > threshold {
(if single_ns < 1.0 { 1.0 } else { single_ns })
if trial.to_double() * single_ns > threshold {
break target_batch_size
}
continue trial * 2
Expand All @@ -41,7 +41,7 @@ fn iter_count(name? : String, inner : () -> Unit, count : UInt) -> Summary {
target_batch_size.ceil().to_int()
}
let samples = Array::makei(count, _ => {
iter_n_microseconds(inner, batch_size) / batch_size.to_double()
iter_n_nanoseconds(inner, batch_size) / batch_size.to_double()
})
samples.sort()
winsorize(sorted_data=samples, 5.0)
Expand Down
2 changes: 1 addition & 1 deletion bench/monotonic_clock_js.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ pub extern "js" fn monotonic_clock_start() -> Timestamp =

///|
pub extern "js" fn monotonic_clock_end(ts : Timestamp) -> Double =
#| (ts) => (performance.now() - ts) * 1000.0
#| (ts) => (performance.now() - ts) * 1000000.0
10 changes: 5 additions & 5 deletions bench/monotonic_clock_wasm.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ fn instant_elapsed_as_secs_f64(x : Timestamp) -> Double = "__moonbit_time_unstab
pub fn monotonic_clock_start() -> Timestamp = "__moonbit_time_unstable" "instant_now"

///|
/// Compute elapsed time in microseconds since `ts`.
/// Compute elapsed time in nanoseconds since `ts`.
///
/// Parameters:
///
/// - `ts`: a timestamp created by `monotonic_clock_start`.
///
/// Returns elapsed time in microseconds.
/// Returns elapsed time in nanoseconds.
///
/// Example:
///
Expand All @@ -60,11 +60,11 @@ pub fn monotonic_clock_start() -> Timestamp = "__moonbit_time_unstable" "instant
/// x = x + 1
/// }
/// ignore(x)
/// let elapsed_us = @bench.monotonic_clock_end(start)
/// inspect(elapsed_us >= 0.0, content="true")
/// let elapsed_ns = @bench.monotonic_clock_end(start)
/// inspect(elapsed_ns >= 0.0, content="true")
/// }
/// ```
pub fn monotonic_clock_end(ts : Timestamp) -> Double {
let elapsed_secs : Double = instant_elapsed_as_secs_f64(ts)
elapsed_secs * 1000000.0
elapsed_secs * 1000000000.0
}
Loading