Skip to content

Use UInt64 to track iteration count during warm-up calculation in Benchmark::IPS#15780

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
syeopite:benchmark-ips-overflow
May 19, 2025
Merged

Use UInt64 to track iteration count during warm-up calculation in Benchmark::IPS#15780
straight-shoota merged 2 commits intocrystal-lang:masterfrom
syeopite:benchmark-ips-overflow

Conversation

@syeopite
Copy link
Contributor

If the warm-up duration is long enough then the iteration count in run_warmup can easily exceed the 32 bit integer limit in a little under a minute resulting in an OverflowError.

This PR just change the tracker variable to an unsigned 64 bit integer so that an overflow is pretty much impossible

You can reproduce the overflow with:

require "benchmark"

# 100 seconds to be safe but I always get an overflow ~ 40 seconds in
Benchmark.ips(warmup: 100.seconds, &.report("") {  }) 

I don't think there's any practical reasons someone would set the warmup duration to be that high but I don't think Benchmark.ips should fail this easily either.

If the warm-up duration is too long then the iteration count in
`run_warmup` can easily exceed the 32 bit integer limit in a little
under a minute resulting in an `OverflowError`.

This commit just change the tracker variable to an unsigned 64 bit
integer so that an overflow is pretty much impossible
Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:benchmark labels May 16, 2025
@straight-shoota straight-shoota added this to the 1.17.0 milestone May 16, 2025
@syeopite syeopite changed the title Use Int64 to track iteration count during warm-up calculation in Benchmark::IPS Use UInt64 to track iteration count during warm-up calculation in Benchmark::IPS May 17, 2025
@straight-shoota straight-shoota merged commit fc444b6 into crystal-lang:master May 19, 2025
36 checks passed
@syeopite syeopite deleted the benchmark-ips-overflow branch May 20, 2025 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:benchmark

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants