fastruby / fast-ruby

:dash: Writing Fast Ruby :heart_eyes: -- Collect Common Ruby idioms.
https://github.com/fastruby/fast-ruby
5.67k stars 376 forks source link

String concat contains dummy comparision. #158

Open alekseyl opened 6 years ago

alekseyl commented 6 years ago

You are doing your test without any randomization and length variety, so results are incorrect completely.

You simply just testing how fast are 'foo' and 'bar' concatenation, and only

Live case scenario looks more like this:

# strings length is a ( 100 + rand( 100 ) ), i.e. short strings max difference around double 

 String#append:   439150.7 i/s
            String#+:   419672.1 i/s - same-ish: difference falls within error
  "#{'foo'}#{'bar'}":   365799.6 i/s - 1.20x  slower
       String#concat:   353912.5 i/s - 1.24x  slower

 # strings length is a  (1000 + rand( 1000) ) , i.e. longer strings max difference around double

            String#+:   298633.5 i/s
   String#append:   284566.8 i/s - same-ish: difference falls within error
  "#{'foo'}#{'bar'}":   267548.4 i/s - 1.12x  slower
    String#concat:   215419.4 i/s - 1.39x  slower

# string length is a ( 100 + rand(1000) ), i.e. highly variable length
             String#+:   388205.2 i/s
   String#append:   357669.3 i/s - 1.09x  slower
  "#{'foo'}#{'bar'}":   331617.7 i/s - 1.17x  slower
    String#concat:   286717.8 i/s - 1.35x  slower

# string length is a 1 + rand 1000, i.e. random string
             String#+:   385180.8 i/s
   String#append:   373926.8 i/s - same-ish: difference falls within error
  "#{'foo'}#{'bar'}":   341496.6 i/s - 1.13x  slower
   String#concat:   297988.1 i/s - 1.29x  slower

OK

# This is completely oposite to
         "foo" "bar":  5369594.5 i/s
 "#{'foo'}#{'bar'}":  5181745.7 i/s - same-ish: difference falls within error
   String#append:  3075719.2 i/s - 1.75x slower
    String#concat:  3016703.5 i/s - 1.78x slower
            String#+:  2977282.7 i/s - 1.80x slower

Also you didn't test on multiple string concat, where actually "#{'foo'} #{'bar'} #{'shines'}".

nirvdrum commented 6 years ago

Pull requests are always appreciated. The problem with randomized data, of course, is that you don't get reproducible results. At the very least, the seed should be specified so results could be deterministic, IMHO.

alekseyl commented 6 years ago

The problem with randomized data, of course, is that you don't get reproducible results.

Randomization must be appropriate to the test, in that case results are reproducible

mateusdeap commented 1 year ago

Hmm this is interesting, actually. Although if we're going to randomize things, I also do believe we'd need things to be averaged out.

Did you test multiple runs of your code, @alekseyl?

Also, if you can, feel free to open a PR and we can continue discussions there.

alekseyl commented 1 year ago

Hmm this is interesting, actually. Although if we're going to randomize things, I also do believe we'd need things to be averaged out.

Did you test multiple runs of your code, @alekseyl?

Also, if you can, feel free to open a PR and we can continue discussions there.

Hi @mateusdeap, here is the PR for you guys to review and check on your own: https://github.com/fastruby/fast-ruby/pull/216/files