This project is archived and is in readonly mode.
Autosave Assocation Needlessly Uses eval()
Reported by Comron Sattari | October 26th, 2009 @ 09:56 PM | in 2.3.6
The blog post at http://blog.pluron.com/2008/02/rails-faster-as.html talks about using String based callbacks in ActiveRecord and how they affect performance through usage of eval() and Kernel.binding. This has been patched in Rails
However I recently found that autosave_association.rb can make use of a similar patch, which I have attached.
Comments and changes to this ticket
-
Eloy Duran December 30th, 2009 @ 07:38 PM
- Assigned user set to “Eloy Duran”
- Milestone set to “2.3.6”
-
Eloy Duran December 31st, 2009 @ 11:01 AM
- State changed from “new” to “invalid”
It's true that evaling a string should be avoided. However, the strings your patch touches are not being evaled. There's only one place in autosave_association.rb that uses eval, module_eval actually, which isn't bad because it's used to define a method, which is faster than using define_method and ok in this case. HTH
-
Comron Sattari December 31st, 2009 @ 04:15 PM
I should have been clearer, it is the callback chain that uses the evel. In this case, after_create, after_update and validate all add these strings to the callback chain, and then evaluate_method in callbacks.rb uses eval() in the callback is a string, and send if it is a symbol. I found the problem while profiling my app.
Please let me know if I'm misunderstanding something.
-
Eloy Duran December 31st, 2009 @ 04:23 PM
- State changed from “invalid” to “open”
Ah yes, you're right. Will fix! Thanks for the update :)
-
Eloy Duran January 7th, 2010 @ 04:24 PM
- State changed from “open” to “verified”
-
Repository January 7th, 2010 @ 07:15 PM
- State changed from “verified” to “resolved”
(from [f866ced24ac9cdece7801f50ec79cdbe3ff5ad59]) Don't use strings for callbacks, as these will be evaled. Rather use symbols, which uses a direct method dispatch.
Patch by Comron Sattari. [#3429 state:resolved]
http://github.com/rails/rails/commit/f866ced24ac9cdece7801f50ec79cd... -
Repository January 7th, 2010 @ 07:32 PM
(from [4b7a439bd17e6d9e8567758ee6a0da13447c3229]) Don't use strings for callbacks, as these will be evaled. Rather use symbols, which uses a direct method dispatch.
Patch by Comron Sattari. [#3429 state:resolved]
http://github.com/rails/rails/commit/4b7a439bd17e6d9e8567758ee6a0da...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Comron Sattari
Eloy Duran
Attachments
Referenced by
-
3429
Autosave Assocation Needlessly Uses eval()
Patch by Comron Sattari. [#3429 state:resolved]
http://gi...
-
3429
Autosave Assocation Needlessly Uses eval()
Patch by Comron Sattari. [#3429 state:resolved]
http://gi...