{"action":"created","issue":{"url":"https://api.github.com/repos/rails/rails/issues/21362","labels_url":"https://api.github.com/repos/rails/rails/issues/21362/labels{/name}","comments_url":"https://api.github.com/repos/rails/rails/issues/21362/comments","events_url":"https://api.github.com/repos/rails/rails/issues/21362/events","html_url":"https://github.com/rails/rails/pull/21362","id":102928626,"number":21362,"title":"Do not allocate new strings for every single run_callbacks call","user":{"login":"pik","id":10236920,"avatar_url":"https://avatars.githubusercontent.com/u/10236920?v=3","gravatar_id":"","url":"https://api.github.com/users/pik","html_url":"https://github.com/pik","followers_url":"https://api.github.com/users/pik/followers","following_url":"https://api.github.com/users/pik/following{/other_user}","gists_url":"https://api.github.com/users/pik/gists{/gist_id}","starred_url":"https://api.github.com/users/pik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pik/subscriptions","organizations_url":"https://api.github.com/users/pik/orgs","repos_url":"https://api.github.com/users/pik/repos","events_url":"https://api.github.com/users/pik/events{/privacy}","received_events_url":"https://api.github.com/users/pik/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2015-08-25T01:33:49Z","updated_at":"2015-08-28T03:25:19Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/rails/rails/pulls/21362","html_url":"https://github.com/rails/rails/pull/21362","diff_url":"https://github.com/rails/rails/pull/21362.diff","patch_url":"https://github.com/rails/rails/pull/21362.patch"},"body":"@schneems \r\nPull request for addressing issue b) from https://groups.google.com/forum/#!topic/rubyonrails-core/NONkf9jLJjM\r\n\r\nFor a retrieval of 729 ActiveRecord objects (6 enum fields, created_at, updated_at) fetched from a postgres database over 8000 strings were allocated. Of these 2916 were strings allocated by ActiveSupport callbacks.rb - although no callbacks were run.\r\n\r\nThese are (measured with memory_profiler gem):\r\n\r\n```\r\n729  \"_initialize_callbacks\"\r\n729  /Users/user_name/.rvm/gems/ruby-2.2.2/gems/activesupport-4.2.3/lib/active_support/callbacks.rb:81\r\n\r\n729  \"_find_callbacks\"\r\n729  /Users/user_name/.rvm/gems/ruby-2.2.2/gems/activesupport-4.2.3/lib/active_support/callbacks.rb:81\r\n\r\n729  \"initialize\"\r\n729  /Users/user_name/.rvm/gems/ruby-2.2.2/gems/activesupport-4.2.3/lib/active_support/callbacks.rb:81\r\n\r\n729  \"find\"\r\n729  /Users/user_name/.rvm/gems/ruby-2.2.2/gems/activesupport-4.2.3/lib/active_support/callbacks.rb:81\r\n```\r\n\r\nThis patch intends to freeze the callbacks strings in a class variable for all Classes extending the ActiveSupport::Callbacks concern. This should result in a tangible improvement memory-management wise, as well as small speed up to the current code:\r\n\r\n```\r\n$_callbacks_store = {}\r\ndef callbacks_store(kind)\r\n  $_callbacks_store[kind] ? $_callbacks_store[kind] : $_callbacks_store[kind]  = \"_#{kind}_callbacks\".freeze\r\nend\r\n\r\nkind = :initialize\r\nBenchmark.ips do |x|\r\n    x.report('use store') {callbacks_store(kind)}\r\n    x.report('allocate new') {\"_#{kind}_callbacks\"}\r\nend\r\n\r\nCalculating -------------------------------------\r\n           use store    54.817k i/100ms\r\n        allocate new    47.754k i/100ms\r\n-------------------------------------------------\r\n           use store      5.942M (± 8.0%) i/s -     29.492M\r\n        allocate new      2.800M (±11.4%) i/s -     13.849M\r\n```\r\n\r\nUnfortunately since run_callbacks :my_callback is entirely dynamic, there seems to be no way to predict or establish which calls could be made within the context of a running application (the more costly alternative being perhaps to freeze all callback strings when define_callbacks is called).\r\n\r\nThe cost for most Rails apps would probably be somewhere between 10-30 allocated strings for the lifetime of the App depending on their use of callbacks with ActiveRecord [registering the most](https://github.com/rails/rails/blob/master/activerecord%2Flib%2Factive_record%2Fcallbacks.rb#L274).\r\n"},"comment":{"url":"https://api.github.com/repos/rails/rails/issues/comments/135623150","html_url":"https://github.com/rails/rails/pull/21362#issuecomment-135623150","issue_url":"https://api.github.com/repos/rails/rails/issues/21362","id":135623150,"user":{"login":"thedarkone","id":15688,"avatar_url":"https://avatars.githubusercontent.com/u/15688?v=3","gravatar_id":"","url":"https://api.github.com/users/thedarkone","html_url":"https://github.com/thedarkone","followers_url":"https://api.github.com/users/thedarkone/followers","following_url":"https://api.github.com/users/thedarkone/following{/other_user}","gists_url":"https://api.github.com/users/thedarkone/gists{/gist_id}","starred_url":"https://api.github.com/users/thedarkone/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thedarkone/subscriptions","organizations_url":"https://api.github.com/users/thedarkone/orgs","repos_url":"https://api.github.com/users/thedarkone/repos","events_url":"https://api.github.com/users/thedarkone/events{/privacy}","received_events_url":"https://api.github.com/users/thedarkone/received_events","type":"User","site_admin":false},"created_at":"2015-08-28T03:25:19Z","updated_at":"2015-08-28T03:25:19Z","body":"@pik on `master` branch, there is no relevant code using `run_callbacks(kind, &block)`, instead [Rails is **already** doing](https://github.com/rails/rails/blob/2a6071a5e81cbc3b83625f5e6d5aff61c69f1720/activerecord/lib/active_record/callbacks.rb#L293-L314) what I was ranting about that **it should** do (see 3rd paragraph in my first reply here). The method you are trying to speed up, is (almost) never used by Rails."}}