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

Recommended fetch with block over args as faster #37

Open
schneems opened this issue Oct 4, 2016 · 7 comments
Open

Recommended fetch with block over args as faster #37

schneems opened this issue Oct 4, 2016 · 7 comments

Comments

@schneems
Copy link

schneems commented Oct 4, 2016

From rails/rails#26599 i'm seeing that's not the case. Btw, this is a really cool project.

cc/ @timrogers

@DamirSvrtan
Copy link
Owner

Thnx @schneems, I'll take a look at it, however I think I will have to make this project ruby-version specific since it seems that speed varies from version to version.

@monfresh
Copy link

FWIW, I just tried Ruby 1.9.3, 2.3.1, 2.3.3, and 2.4.0, and in all those versions, the argument version was faster than the block version.

@gagalago
Copy link

see this note on https://github.com/JuanitoFatas/fast-ruby#hashfetch-with-argument-vs-hashfetch--block-code
it seems from that that we should make a distinction between a constant argument and method argument.

@DamirSvrtan
Copy link
Owner

@gagalago would you be open to creating a PR with a fix for this?

Best,
Damir

@gagalago
Copy link

I can check but I will not do any promise. I am not familiar with code of fasterer (neither from other linters) so I don't know if I will be able to do it.

@DamirSvrtan
Copy link
Owner

Sure thing! Lemme know if you need any help figuring anything around the codebase, probably a good start would be checking this method over here: https://github.com/DamirSvrtan/fasterer/blob/master/lib/fasterer/scanners/method_call_scanner.rb#L116

@connicov1
Copy link

Hey guys, if you don't mind I created a small PR with the fix
#93
Please check it when you have time, probably did something wrong...

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

5 participants