zombocom / derailed_benchmarks

Go faster, off the Rails - Benchmarks for your whole Rails app
2.98k stars 144 forks source link

Parsing issues when timing output to log is large #225

Open rsanheim opened 2 years ago

rsanheim commented 2 years ago

I've been hitting an issue where the benchmark label derailed requests has no whitespace before the following decimal, so stats_for_file fails:

[~/src/monograph/monograph/apps/api (small-gantt-perf-1)]> PATH_TO_HIT=/projects REFS_TO_TEST=main,small-gantt-perf-1 TEST_COUNT=500 DERAILED_SCRIPT_COUNT=20 script/bench compare
You can tail the log output in tmp/compare_branches/2022-10-28-19-28-1667003335-099131000 to follow progress...

Testing 1: 1600d42db: Speed Up Role Drag/Drop (#974)
Testing 2: cb3cb7b7f: Merge branch 'small-gantt-perf-1' of https://github.com/monographhq/monograph into small-gantt-perf-1

Sample: 1/20 iterations per sample: 500
Intermediate result
Resetting git dir of '/Users/rsanheim/src/monograph/monograph' to "small-gantt-perf-1"
rake aborted!
TypeError: Problem with file #<Pathname:tmp/compare_branches/2022-10-28-19-28-1667003335-099131000/main.bench.txt>:
500 derailed requests461.941212   3.555428 465.500431 (790.344929)

can't convert nil into BigDecimal
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_for_file.rb:56:in `BigDecimal'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_for_file.rb:56:in `block in load_file!'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_for_file.rb:53:in `foreach'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_for_file.rb:53:in `each_line'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_for_file.rb:53:in `load_file!'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_for_file.rb:32:in `call'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_from_dir.rb:61:in `each'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/stats_from_dir.rb:61:in `call'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/tasks.rb:70:in `ensure in block (2 levels) in <top (required)>'
/Users/rsanheim/src/monograph/monograph/apps/api/vendor/gems/derailed_benchmarks-2.1.2/lib/derailed_benchmarks/tasks.rb:77:in `block (2 levels) in <top (required)>'

I believe the below line in the log breaks this regex:

500 derailed requests461.941212   3.555428 465.500431 (790.344929)

We have this vendor'ed in our project, so once I confirm a fix I can probably get a PR up posted to fix this. Just wanted to raise the issue in case others have hit it and I'm missing some other factor here.

rsanheim commented 2 years ago

Okay I was wrong. The regex expects leading whitespace in the real reporting number, the one in parents. In these results, I guess cuz they are so large, there is no leading whitespace, just (800.234) or whatever.