Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String concat contains dummy comparision. #158

Open
alekseyl opened this issue Jun 12, 2018 · 4 comments
Open

String concat contains dummy comparision. #158

alekseyl opened this issue Jun 12, 2018 · 4 comments

Comments

@alekseyl
Copy link
Contributor

alekseyl commented Jun 12, 2018

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants