This project is archived and is in readonly mode.
Autosave Assocation Needlessly Uses eval()
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.
-
autosave.diff
811 Bytes
-
Eloy Duran- Assigned user set to Eloy Duran
- Milestone set to 2.3.6
-
Eloy Duran
- 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 SattariI 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
- State changed from invalid to open
Ah yes, you're right. Will fix! Thanks for the update :)
-
Eloy Duran
- State changed from open to verified
-
Repository
- 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 Autosave Assocation Needlessly Uses eval() state:resolved]
http://github.com/rails/rails/commit/f866ced24ac9cdece7801f50ec79cd... -
Repository
(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 Autosave Assocation Needlessly Uses eval() state:resolved]
http://github.com/rails/rails/commit/4b7a439bd17e6d9e8567758ee6a0da...