search

evanphx / benchmark-ips

Provides iteration per second benchmarking for Ruby
MIT License
1.72k stars 97 forks source link

Report to stream and iron out compare abstraction #89

Closed kbrock closed 4 years ago

kbrock commented 5 years ago

This unifies the double @stdout and @suite calls. It also allows a user to choose a different output.

Addresses #81 for warmup and benchmark but not compare!

Outstanding:

Changing compare is not as easy, the output needs to be passed in:

  1. Pass in the stream from one of the @outs
  2. Pass in a stream to config and use that throughout
  3. Enhance StdoutReport and change compare to use the @outs.

Let me know which you approach you prefer. I can fixup this PR or just make another.

Sample Code

require "benchmark/ips"

my_stream = StringIO.new
Benchmark.ips do |x|
  x.config(:quiet => true, :suite => ::Benchmark::IPS::Job::StdoutReport.new(my_stream))

  x.report("mul") { 2 * 2 * 2 * 2 * 2 * 2 * 2 * 2 }
  x.report("pow") { 2 ** 8 }

  # x.compare!
end

puts "results: #{my_stream.string}"
kbrock commented 5 years ago

Haven't had to type an encoding in eons...

# encoding: utf-8
ioquatix commented 5 years ago

It looks great.

kbrock commented 5 years ago

I removed compare from the mix. just made it too complicated.

Instead, I can create a PR just for compare but guidance is suggested.

evanphx commented 5 years ago

I merged #90 so this has a conflict now. As for how to deal with compare!, I’d prefer to delegate it to the report itself.

kbrock commented 5 years ago

kicking. odd bundler errors

kbrock commented 5 years ago

trying again. seeing if there was a blip, or something is wrong at the core

nateberkopec commented 4 years ago

@kbrock Still want to get this merged? I can help.

kbrock commented 4 years ago

Hi @nateberkopec

I don't think I'll use this functionality. I thought it helped clean up the code but hard to say that with +73/-24

If you like the changes, then I'll make what ever updates you want. Otherwise I can close.

nateberkopec commented 4 years ago

Please leave open for now - I'll take another look!

kbrock commented 4 years ago

@nateberkopec let me know if you want me to move stuff around here or introduce just part for another PR

Feel free to copy anything or ignore. Just trying to help

nateberkopec commented 3 years ago

I don't really know why I closed this tbh

kbrock commented 3 years ago

lol

This was in reaction to wanting all output to be via a consistent channel. A simple @out=STDOUT would probably have been good enough.

The implementation was a little buggy as determining whether to output some of the output in the various modes did not quite match the current behavior.

Also, to be honest - the feature was not for me. I use other means of capturing the metrics