Skip to content
Browse files

Hash#symbolize_keys(!) optimizations

[#3891 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
1 parent 82deaf5 commit 2060977b767061a42eb8db2d5c3a30d205a94123 @methodmissing methodmissing committed with jeremy
Showing with 1 addition and 1 deletion.
  1. +1 −1 activesupport/lib/active_support/core_ext/hash/keys.rb
View
2 activesupport/lib/active_support/core_ext/hash/keys.rb
@@ -22,7 +22,7 @@ def symbolize_keys
# to +to_sym+.
def symbolize_keys!
keys.each do |key|
- self[(key.to_sym rescue key) || key] = delete(key)
+ self[(key.to_sym rescue key)] = delete(key) if key.respond_to?(:to_sym) && !key.is_a?(Fixnum)
end
self
end

6 comments on commit 2060977

@sobrinho

rescue is needed using respond_to?

@matthewrudy

presumably

keys.each do |key|
unless key.is_a?(Fixnum) || key.is_a?(Symbol)
if key.respond_to?(:to_sym)
self[key.to_sym] = delete(key)
end
end
end

gets you what you want.
Are we really coding for :to_sym not working
(perhaps because it has arity > 0)

@yfeldblum

In various languages, try-catch blocks are cheap while throwing and type-checking/casting are a little bit more expensive. In these languages, it may actually be faster to use the try-catch than to use type-checking. Obviously, if an exception is thrown that a type-check/cast would have prevented, all that performance increase disappears - so that kind of technique is used carefully.

Any idea what the difference in performance between the two techniques is in Ruby?

@sobrinho

I think this can be:

self[key.to_sym] = delete(key) if key.respond_to?(:to_sym) && !key.is_a?(Fixnum)

Not?

@libc

@yfeldblum raise/rescue is not cheap in mri. (e.g. http://pastie.org/pastes/807345 we had to switch to FFI because raise causes syscall and system time goes up).

@yfeldblum

Raising is absolutely going to be expensive. But what about code that does define a rescue block but that doesn't end up raising? Suppose all the code in the try part runs successfully - will the mere existence of a rescue block slow down the try block?

Please sign in to comment.
Something went wrong with that request. Please try again.