{"action":"created","comment":{"url":"https://api.github.com/repos/rails/rails/pulls/comments/41516807","id":41516807,"diff_hunk":"@@ -75,32 +62,24 @@ def options\n         private\n \n         def associated_records_by_owner(preloader)\n-          owners_map = owners_by_key\n-          owner_keys = owners_map.keys.compact\n-\n-          # Each record may have multiple owners, and vice-versa\n-          records_by_owner = owners.each_with_object({}) do |owner,h|\n-            h[owner] = []\n-          end\n-\n-          if owner_keys.any?\n-            # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000)\n-            # Make several smaller queries if necessary or make one query if the adapter supports it\n-            sliced  = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size)\n-\n-            records = load_slices sliced\n-            records.each do |record, owner_key|\n-              owners_map[owner_key].each do |owner|\n-                records_by_owner[owner] << record\n-              end\n-            end\n+          records = load_records\n+          owners.each_with_object({}) do |owner, result|\n+            result[owner] = records[convert_key(owner[owner_key_name])] || []\n           end\n+        end\n \n-          records_by_owner\n+        def owner_keys\n+          @owner_keys ||= owners.map do |owner|\n+            owner[owner_key_name]\n+          end.tap {|z| z.uniq!; z.compact!}\n         end\n \n         def key_conversion_required?\n-          association_key_type != owner_key_type\n+          @key_conversion_required ||= association_key_type != owner_key_type\n+        end\n+\n+        def convert_key(key)\n+          key_conversion_required? ? key.to_s : key","path":"activerecord/lib/active_record/associations/preloader/association.rb","position":69,"original_position":69,"commit_id":"caa35912ecd05c366ea2a5fcb57357a4546b1eb5","original_commit_id":"caa35912ecd05c366ea2a5fcb57357a4546b1eb5","user":{"login":"bogdan","id":122436,"avatar_url":"https://avatars.githubusercontent.com/u/122436?v=3","gravatar_id":"","url":"https://api.github.com/users/bogdan","html_url":"https://github.com/bogdan","followers_url":"https://api.github.com/users/bogdan/followers","following_url":"https://api.github.com/users/bogdan/following{/other_user}","gists_url":"https://api.github.com/users/bogdan/gists{/gist_id}","starred_url":"https://api.github.com/users/bogdan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bogdan/subscriptions","organizations_url":"https://api.github.com/users/bogdan/orgs","repos_url":"https://api.github.com/users/bogdan/repos","events_url":"https://api.github.com/users/bogdan/events{/privacy}","received_events_url":"https://api.github.com/users/bogdan/received_events","type":"User","site_admin":false},"body":"Why is that? So that I know the code style....","created_at":"2015-10-08T14:07:08Z","updated_at":"2015-10-08T14:07:08Z","html_url":"https://github.com/rails/rails/pull/21918#discussion_r41516807","pull_request_url":"https://api.github.com/repos/rails/rails/pulls/21918","_links":{"self":{"href":"https://api.github.com/repos/rails/rails/pulls/comments/41516807"},"html":{"href":"https://github.com/rails/rails/pull/21918#discussion_r41516807"},"pull_request":{"href":"https://api.github.com/repos/rails/rails/pulls/21918"}}},"pull_request":{"url":"https://api.github.com/repos/rails/rails/pulls/21918","id":47149559,"html_url":"https://github.com/rails/rails/pull/21918","diff_url":"https://github.com/rails/rails/pull/21918.diff","patch_url":"https://github.com/rails/rails/pull/21918.patch","issue_url":"https://api.github.com/repos/rails/rails/issues/21918","number":21918,"state":"open","locked":false,"title":"Refactored association preloader for performance","user":{"login":"bogdan","id":122436,"avatar_url":"https://avatars.githubusercontent.com/u/122436?v=3","gravatar_id":"","url":"https://api.github.com/users/bogdan","html_url":"https://github.com/bogdan","followers_url":"https://api.github.com/users/bogdan/followers","following_url":"https://api.github.com/users/bogdan/following{/other_user}","gists_url":"https://api.github.com/users/bogdan/gists{/gist_id}","starred_url":"https://api.github.com/users/bogdan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bogdan/subscriptions","organizations_url":"https://api.github.com/users/bogdan/orgs","repos_url":"https://api.github.com/users/bogdan/repos","events_url":"https://api.github.com/users/bogdan/events{/privacy}","received_events_url":"https://api.github.com/users/bogdan/received_events","type":"User","site_admin":false},"body":"* less arrays created: using `group_by` instead of array of arrays of two elements\r\n\r\nFrom:\r\nhttps://github.com/rails/rails/pull/21918/files#diff-83ce56e0f856f2c8d7ceb7fab4eb4c2bL92\r\nTo:\r\nhttps://github.com/rails/rails/pull/21918/files#diff-83ce56e0f856f2c8d7ceb7fab4eb4c2bR66\r\n\r\n* less complexity with only one level of nesting in loop\r\n\r\nhttps://github.com/rails/rails/pull/21918/files#diff-83ce56e0f856f2c8d7ceb7fab4eb4c2bR101\r\n\r\nBenchmark:\r\nhttps://gist.github.com/7b486f5852f4f833ce37\r\n\r\n\r\n``` \r\n                    user     system      total        real\r\n------------------1000 iterations 1 associated for 10 given\r\nAfter patch:    0.710000   0.050000   0.760000 (  0.757919)\r\nBefore patch:   0.720000   0.050000   0.770000 (  0.768007)\r\nImprovement: 1%\r\n\r\n-----------------1000 iterations 10 associated for 10 given\r\nAfter patch:    2.270000   0.120000   2.390000 (  2.387669)\r\nBefore patch:   2.450000   0.120000   2.570000 (  2.568925)\r\nImprovement: 7%\r\n\r\n-----------------100 iterations 100 associated for 10 given\r\nAfter patch:    1.770000   0.070000   1.840000 (  1.845977)\r\nBefore patch:   1.990000   0.080000   2.070000 (  2.067470)\r\nImprovement: 11%\r\n\r\n------------------100 iterations 1 associated for 100 given\r\nAfter patch:    0.370000   0.010000   0.380000 (  0.378398)\r\nBefore patch:   0.360000   0.010000   0.370000 (  0.370559)\r\nImprovement: -2%\r\n\r\n-----------------100 iterations 10 associated for 100 given\r\nAfter patch:    1.930000   0.080000   2.010000 (  2.013075)\r\nBefore patch:   2.140000   0.080000   2.220000 (  2.224573)\r\nImprovement: 10%\r\n\r\n-----------------10 iterations 100 associated for 100 given\r\nAfter patch:    1.840000   0.080000   1.920000 (  1.928696)\r\nBefore patch:   2.040000   0.090000   2.130000 (  2.132890)\r\nImprovement: 10%\r\n\r\n```\r\n\r\nSlight performance present when loading only one associated record, but I think this is kind of ok.\r\n","created_at":"2015-10-08T13:04:26Z","updated_at":"2015-10-08T14:07:08Z","closed_at":null,"merged_at":null,"merge_commit_sha":"20b96301cfc030150de7691ee180606d1a825af2","assignee":{"login":"sgrif","id":1529387,"avatar_url":"https://avatars.githubusercontent.com/u/1529387?v=3","gravatar_id":"","url":"https://api.github.com/users/sgrif","html_url":"https://github.com/sgrif","followers_url":"https://api.github.com/users/sgrif/followers","following_url":"https://api.github.com/users/sgrif/following{/other_user}","gists_url":"https://api.github.com/users/sgrif/gists{/gist_id}","starred_url":"https://api.github.com/users/sgrif/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sgrif/subscriptions","organizations_url":"https://api.github.com/users/sgrif/orgs","repos_url":"https://api.github.com/users/sgrif/repos","events_url":"https://api.github.com/users/sgrif/events{/privacy}","received_events_url":"https://api.github.com/users/sgrif/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/rails/rails/pulls/21918/commits","review_comments_url":"https://api.github.com/repos/rails/rails/pulls/21918/comments","review_comment_url":"https://api.github.com/repos/rails/rails/pulls/comments{/number}","comments_url":"https://api.github.com/repos/rails/rails/issues/21918/comments","statuses_url":"https://api.github.com/repos/rails/rails/statuses/caa35912ecd05c366ea2a5fcb57357a4546b1eb5","head":{"label":"bogdan:refactor-preloader","ref":"refactor-preloader","sha":"caa35912ecd05c366ea2a5fcb57357a4546b1eb5","user":{"login":"bogdan","id":122436,"avatar_url":"https://avatars.githubusercontent.com/u/122436?v=3","gravatar_id":"","url":"https://api.github.com/users/bogdan","html_url":"https://github.com/bogdan","followers_url":"https://api.github.com/users/bogdan/followers","following_url":"https://api.github.com/users/bogdan/following{/other_user}","gists_url":"https://api.github.com/users/bogdan/gists{/gist_id}","starred_url":"https://api.github.com/users/bogdan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bogdan/subscriptions","organizations_url":"https://api.github.com/users/bogdan/orgs","repos_url":"https://api.github.com/users/bogdan/repos","events_url":"https://api.github.com/users/bogdan/events{/privacy}","received_events_url":"https://api.github.com/users/bogdan/received_events","type":"User","site_admin":false},"repo":{"id":3515719,"name":"rails","full_name":"bogdan/rails","owner":{"login":"bogdan","id":122436,"avatar_url":"https://avatars.githubusercontent.com/u/122436?v=3","gravatar_id":"","url":"https://api.github.com/users/bogdan","html_url":"https://github.com/bogdan","followers_url":"https://api.github.com/users/bogdan/followers","following_url":"https://api.github.com/users/bogdan/following{/other_user}","gists_url":"https://api.github.com/users/bogdan/gists{/gist_id}","starred_url":"https://api.github.com/users/bogdan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bogdan/subscriptions","organizations_url":"https://api.github.com/users/bogdan/orgs","repos_url":"https://api.github.com/users/bogdan/repos","events_url":"https://api.github.com/users/bogdan/events{/privacy}","received_events_url":"https://api.github.com/users/bogdan/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bogdan/rails","description":"Ruby on Rails","fork":true,"url":"https://api.github.com/repos/bogdan/rails","forks_url":"https://api.github.com/repos/bogdan/rails/forks","keys_url":"https://api.github.com/repos/bogdan/rails/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bogdan/rails/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bogdan/rails/teams","hooks_url":"https://api.github.com/repos/bogdan/rails/hooks","issue_events_url":"https://api.github.com/repos/bogdan/rails/issues/events{/number}","events_url":"https://api.github.com/repos/bogdan/rails/events","assignees_url":"https://api.github.com/repos/bogdan/rails/assignees{/user}","branches_url":"https://api.github.com/repos/bogdan/rails/branches{/branch}","tags_url":"https://api.github.com/repos/bogdan/rails/tags","blobs_url":"https://api.github.com/repos/bogdan/rails/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bogdan/rails/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bogdan/rails/git/refs{/sha}","trees_url":"https://api.github.com/repos/bogdan/rails/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bogdan/rails/statuses/{sha}","languages_url":"https://api.github.com/repos/bogdan/rails/languages","stargazers_url":"https://api.github.com/repos/bogdan/rails/stargazers","contributors_url":"https://api.github.com/repos/bogdan/rails/contributors","subscribers_url":"https://api.github.com/repos/bogdan/rails/subscribers","subscription_url":"https://api.github.com/repos/bogdan/rails/subscription","commits_url":"https://api.github.com/repos/bogdan/rails/commits{/sha}","git_commits_url":"https://api.github.com/repos/bogdan/rails/git/commits{/sha}","comments_url":"https://api.github.com/repos/bogdan/rails/comments{/number}","issue_comment_url":"https://api.github.com/repos/bogdan/rails/issues/comments{/number}","contents_url":"https://api.github.com/repos/bogdan/rails/contents/{+path}","compare_url":"https://api.github.com/repos/bogdan/rails/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bogdan/rails/merges","archive_url":"https://api.github.com/repos/bogdan/rails/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bogdan/rails/downloads","issues_url":"https://api.github.com/repos/bogdan/rails/issues{/number}","pulls_url":"https://api.github.com/repos/bogdan/rails/pulls{/number}","milestones_url":"https://api.github.com/repos/bogdan/rails/milestones{/number}","notifications_url":"https://api.github.com/repos/bogdan/rails/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bogdan/rails/labels{/name}","releases_url":"https://api.github.com/repos/bogdan/rails/releases{/id}","created_at":"2012-02-22T15:18:15Z","updated_at":"2015-02-27T15:17:57Z","pushed_at":"2015-10-08T12:56:31Z","git_url":"git://github.com/bogdan/rails.git","ssh_url":"git@github.com:bogdan/rails.git","clone_url":"https://github.com/bogdan/rails.git","svn_url":"https://github.com/bogdan/rails","homepage":"http://rubyonrails.org","size":143800,"stargazers_count":1,"watchers_count":1,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"rails:master","ref":"master","sha":"c750ca5da8683833109b5b980d029a918c2c0989","user":{"login":"rails","id":4223,"avatar_url":"https://avatars.githubusercontent.com/u/4223?v=3","gravatar_id":"","url":"https://api.github.com/users/rails","html_url":"https://github.com/rails","followers_url":"https://api.github.com/users/rails/followers","following_url":"https://api.github.com/users/rails/following{/other_user}","gists_url":"https://api.github.com/users/rails/gists{/gist_id}","starred_url":"https://api.github.com/users/rails/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rails/subscriptions","organizations_url":"https://api.github.com/users/rails/orgs","repos_url":"https://api.github.com/users/rails/repos","events_url":"https://api.github.com/users/rails/events{/privacy}","received_events_url":"https://api.github.com/users/rails/received_events","type":"Organization","site_admin":false},"repo":{"id":8514,"name":"rails","full_name":"rails/rails","owner":{"login":"rails","id":4223,"avatar_url":"https://avatars.githubusercontent.com/u/4223?v=3","gravatar_id":"","url":"https://api.github.com/users/rails","html_url":"https://github.com/rails","followers_url":"https://api.github.com/users/rails/followers","following_url":"https://api.github.com/users/rails/following{/other_user}","gists_url":"https://api.github.com/users/rails/gists{/gist_id}","starred_url":"https://api.github.com/users/rails/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rails/subscriptions","organizations_url":"https://api.github.com/users/rails/orgs","repos_url":"https://api.github.com/users/rails/repos","events_url":"https://api.github.com/users/rails/events{/privacy}","received_events_url":"https://api.github.com/users/rails/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/rails/rails","description":"Ruby on Rails","fork":false,"url":"https://api.github.com/repos/rails/rails","forks_url":"https://api.github.com/repos/rails/rails/forks","keys_url":"https://api.github.com/repos/rails/rails/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rails/rails/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rails/rails/teams","hooks_url":"https://api.github.com/repos/rails/rails/hooks","issue_events_url":"https://api.github.com/repos/rails/rails/issues/events{/number}","events_url":"https://api.github.com/repos/rails/rails/events","assignees_url":"https://api.github.com/repos/rails/rails/assignees{/user}","branches_url":"https://api.github.com/repos/rails/rails/branches{/branch}","tags_url":"https://api.github.com/repos/rails/rails/tags","blobs_url":"https://api.github.com/repos/rails/rails/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rails/rails/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rails/rails/git/refs{/sha}","trees_url":"https://api.github.com/repos/rails/rails/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rails/rails/statuses/{sha}","languages_url":"https://api.github.com/repos/rails/rails/languages","stargazers_url":"https://api.github.com/repos/rails/rails/stargazers","contributors_url":"https://api.github.com/repos/rails/rails/contributors","subscribers_url":"https://api.github.com/repos/rails/rails/subscribers","subscription_url":"https://api.github.com/repos/rails/rails/subscription","commits_url":"https://api.github.com/repos/rails/rails/commits{/sha}","git_commits_url":"https://api.github.com/repos/rails/rails/git/commits{/sha}","comments_url":"https://api.github.com/repos/rails/rails/comments{/number}","issue_comment_url":"https://api.github.com/repos/rails/rails/issues/comments{/number}","contents_url":"https://api.github.com/repos/rails/rails/contents/{+path}","compare_url":"https://api.github.com/repos/rails/rails/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rails/rails/merges","archive_url":"https://api.github.com/repos/rails/rails/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rails/rails/downloads","issues_url":"https://api.github.com/repos/rails/rails/issues{/number}","pulls_url":"https://api.github.com/repos/rails/rails/pulls{/number}","milestones_url":"https://api.github.com/repos/rails/rails/milestones{/number}","notifications_url":"https://api.github.com/repos/rails/rails/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rails/rails/labels{/name}","releases_url":"https://api.github.com/repos/rails/rails/releases{/id}","created_at":"2008-04-11T02:19:47Z","updated_at":"2015-10-08T13:56:45Z","pushed_at":"2015-10-08T14:06:48Z","git_url":"git://github.com/rails/rails.git","ssh_url":"git@github.com:rails/rails.git","clone_url":"https://github.com/rails/rails.git","svn_url":"https://github.com/rails/rails","homepage":"http://rubyonrails.org","size":462590,"stargazers_count":27955,"watchers_count":27955,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":11266,"mirror_url":null,"open_issues_count":889,"forks":11266,"open_issues":889,"watchers":27955,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/rails/rails/pulls/21918"},"html":{"href":"https://github.com/rails/rails/pull/21918"},"issue":{"href":"https://api.github.com/repos/rails/rails/issues/21918"},"comments":{"href":"https://api.github.com/repos/rails/rails/issues/21918/comments"},"review_comments":{"href":"https://api.github.com/repos/rails/rails/pulls/21918/comments"},"review_comment":{"href":"https://api.github.com/repos/rails/rails/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/rails/rails/pulls/21918/commits"},"statuses":{"href":"https://api.github.com/repos/rails/rails/statuses/caa35912ecd05c366ea2a5fcb57357a4546b1eb5"}}}}