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

Speed up code and avoid unnecessary MatchData objects #21155

Merged
merged 1 commit into from Aug 7, 2015

Conversation

Projects
None yet
4 participants
@AaronLasseigne
Contributor

AaronLasseigne commented Aug 7, 2015

This runs faster and creates fewer objects.

A more detailed discussion including some benchmarks can be found here: schneems@1bf50ba#commitcomment-12572839

cc: @schneems

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Aug 7, 2015

Member

Thanks! You rock!

Member

schneems commented Aug 7, 2015

Thanks! You rock!

schneems added a commit that referenced this pull request Aug 7, 2015

Merge pull request #21155 from AaronLasseigne/fewer_objects
Speed up code and avoid unnecessary MatchData objects

@schneems schneems merged commit 261b275 into rails:master Aug 7, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@AaronLasseigne

This comment has been minimized.

Show comment
Hide comment
@AaronLasseigne

AaronLasseigne Aug 7, 2015

Contributor

Woot! Thanks for the merge.

Contributor

AaronLasseigne commented Aug 7, 2015

Woot! Thanks for the merge.

@@ -49,7 +49,7 @@ def add(words)
end
def uncountable?(str)
@regex_array.detect {|regex| regex.match(str) }
@regex_array.any? { |regex| str =~ regex }

This comment has been minimized.

@sgrif

sgrif Aug 8, 2015

Member

Should this use Regex#===?

@sgrif

sgrif Aug 8, 2015

Member

Should this use Regex#===?

This comment has been minimized.

@rafaelfranca

rafaelfranca Aug 8, 2015

Member

@schneems already did later.

@rafaelfranca

rafaelfranca Aug 8, 2015

Member

@schneems already did later.

This comment has been minimized.

@sgrif

sgrif Aug 8, 2015

Member

I suppose it's 0 allocations regardless.

@sgrif

sgrif Aug 8, 2015

Member

I suppose it's 0 allocations regardless.

This comment has been minimized.

@sgrif

sgrif Aug 8, 2015

Member

Gotcha. Thanks.

@sgrif

sgrif Aug 8, 2015

Member

Gotcha. Thanks.

This comment has been minimized.

@schneems

schneems Aug 8, 2015

Member

It's debatable which is better, here's another thread on the topic: JuanitoFatas/fast-ruby#62 I think either is fine. Probably didn't need the patch I sent in.

@schneems

schneems Aug 8, 2015

Member

It's debatable which is better, here's another thread on the topic: JuanitoFatas/fast-ruby#62 I think either is fine. Probably didn't need the patch I sent in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment