{"action":"closed","number":17049,"pull_request":{"url":"https://api.github.com/repos/mantidproject/mantid/pulls/17049","id":79015235,"html_url":"https://github.com/mantidproject/mantid/pull/17049","diff_url":"https://github.com/mantidproject/mantid/pull/17049.diff","patch_url":"https://github.com/mantidproject/mantid/pull/17049.patch","issue_url":"https://api.github.com/repos/mantidproject/mantid/issues/17049","number":17049,"state":"closed","locked":false,"title":"Fix performance regression in EventList","user":{"login":"SimonHeybrock","id":12912489,"avatar_url":"https://avatars.githubusercontent.com/u/12912489?v=3","gravatar_id":"","url":"https://api.github.com/users/SimonHeybrock","html_url":"https://github.com/SimonHeybrock","followers_url":"https://api.github.com/users/SimonHeybrock/followers","following_url":"https://api.github.com/users/SimonHeybrock/following{/other_user}","gists_url":"https://api.github.com/users/SimonHeybrock/gists{/gist_id}","starred_url":"https://api.github.com/users/SimonHeybrock/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SimonHeybrock/subscriptions","organizations_url":"https://api.github.com/users/SimonHeybrock/orgs","repos_url":"https://api.github.com/users/SimonHeybrock/repos","events_url":"https://api.github.com/users/SimonHeybrock/events{/privacy}","received_events_url":"https://api.github.com/users/SimonHeybrock/received_events","type":"User","site_admin":false},"body":"@LamarMoore notices a performance regression in `DiffractionFocussing2PerformanceTest` by the `Histogram::YMode` PR.\r\n\r\nThe performance results are here: http://builds.mantidproject.org/view/Tests/job/master_performancetests/Master_branch_performance_tests/AlgorithmsTest.DiffractionFocussing2TestPerformance.test_SNAP_event_six_groups_dontPreserveEvents.htm.\r\n\r\n```\r\n100\t2016-07-26 09:18:57\tf759dc8\tdiff\t\t6.033\t1.69441\t\r\n99\t2016-07-26 08:57:42\t8534a80\tdiff\t\t5.946\t1.69504\t\r\n98\t2016-07-25 13:49:30\tead5760\tdiff\t\t6.108\t1.69646\t\r\n97\t2016-07-25 09:31:56\t6f6ed2d\tdiff\t\t5.925\t1.70133\t\r\n96\t2016-07-25 09:05:06\td4fe7ae\tdiff\t\t7.124\t1.38968\t\r\n95\t2016-07-23 23:53:18\te7901ed\tdiff\t\t7.986\t1.28852\t\r\n94\t2016-07-22 23:22:50\tac3c4dd\tdiff\t\t5.897\t1.70326\t\r\n93\t2016-07-22 23:01:50\t7bdf0f3\tdiff\t\t5.834\t1.69924\t\r\n92\t2016-07-22 22:34:50\ta966d75\tdiff\t\t5.754\t1.67303\t\r\n91\t2016-07-22 15:31:45\ta6eecf7\tdiff\t\t6.324\t1.69839\t\r\n90\t2016-07-22 09:30:12\t717a200\tdiff\t\t3.061\t1.66984\t\r\n89\t2016-07-22 09:06:35\t4ffef1c\tdiff\t\t3.081\t1.66816\t\r\n88\t2016-07-21 23:29:47\td7f8863\tdiff\t\t3.129\t1.65929\t\r\n87\t2016-07-21 23:09:12\td7f8863\tdiff\t\t3.018\t1.66478\t\r\n86\t2016-07-21 13:07:13\t77a2b60\tdiff\t\t3.106\t1.65729\t\r\n85\t2016-07-21 10:52:46\t43096d2\tdiff\t\t3.313\t1.61791\t\r\n84\t2016-07-21 09:59:52\t43096d2\tdiff\t\t3.216\t1.6038\t\r\n83\t2016-07-21 09:34:59\t1c23080\tdiff\t\t3.231\t1.66098\t\r\n82\t2016-07-20 22:20:04\tfa98011\tdiff\t\t3.151\t1.66597\t\r\n81\t2016-07-20 16:16:29\t4b996f4\tdiff\t\t3.257\t1.6633\t\r\n80\t2016-07-15 09:33:23\t589fa35\tdiff\t\t3.051\t1.6786\r\n```\r\n\r\nThis turns out to be due to changes in `EventList` that increased the number of look-ups in the MRU, and also the `shared_ptr` handling. I had not contemplated this as critical during the implementation, but apparently it is.\r\n\r\nHere we reduce the number of look-ups back to the previous amount for `Y` and `E` access.\r\n\r\nFor the typed accessors (`counts()`, `frequencies()`, ...) we are still going the route via `histogram()` with more look-ups.  This could also be fixed but is slightly more complicated. We can do so if we notice more performance issues. This issue can be entirely avoided in clients by working with `EventList::histogram()` and operating based on the returned `Histogram` instead of using the individual access.\r\n\r\n**To test:**\r\n\r\n<!-- Instructions for testing. -->\r\n\r\n- Code review.\r\n- Check DiffractionFocussing2PerformanceTest results and see if they are back to normal.\r\n- No corresponding issue.\r\n- No release notes since the PR that caused this was merged after the last release.\r\n\r\n---\r\n\r\n#### Reviewer ####\r\n\r\nPlease comment on the following ([full description](http://www.mantidproject.org/Individual_Ticket_Testing)):\r\n\r\n##### Code Review #####\r\n\r\n- [x] Is the code of an acceptable quality?\r\n- [ ] Does the code conform to the [coding standards](http://www.mantidproject.org/Coding_Standards)? Is it well structured with small focussed classes/methods/functions?\r\n- [x] Are there unit/system tests in place? Are the unit tests small and test the a class in isolation?\r\n- [ ] If there are changes in the release notes then do they describe the changes appropriately?\r\n\r\n##### Functional Tests #####\r\n\r\n- [x] Do changes function as described? Add comments below that describe the tests performed?\r\n- [ ] How do the changes handle unexpected situations, e.g. bad input?\r\n- [ ] Has the relevant documentation been added/updated?\r\n- [ ] Is user-facing documentation written in a user-friendly manner?\r\n- [ ] Has developer documentation been updated if required?\r\n\r\n- Does everything look good? Comment with the ship it emoji but don't merge. A member of `@mantidproject/gatekeepers` will take care of it.\r\n\r\nCaused by recent changes when introducing Histogram::YMode. Performance\r\nregression was visible in\r\nAlgorithmsTest.DiffractionFocussing2TestPerformance\r\ntest_SNAP_event_six_groups_dontPreserveEvents.\r\n\r\nThe overhead seems to be cause by map lookup in the MRU, which I had\r\npreviously considered insignificant.","created_at":"2016-07-27T12:30:02Z","updated_at":"2016-08-01T13:12:17Z","closed_at":"2016-08-01T13:12:17Z","merged_at":"2016-08-01T13:12:17Z","merge_commit_sha":"34b5c89443d825c10f1f9318b5f2c6968cfcd833","assignee":{"login":"LamarMoore","id":15360272,"avatar_url":"https://avatars.githubusercontent.com/u/15360272?v=3","gravatar_id":"","url":"https://api.github.com/users/LamarMoore","html_url":"https://github.com/LamarMoore","followers_url":"https://api.github.com/users/LamarMoore/followers","following_url":"https://api.github.com/users/LamarMoore/following{/other_user}","gists_url":"https://api.github.com/users/LamarMoore/gists{/gist_id}","starred_url":"https://api.github.com/users/LamarMoore/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/LamarMoore/subscriptions","organizations_url":"https://api.github.com/users/LamarMoore/orgs","repos_url":"https://api.github.com/users/LamarMoore/repos","events_url":"https://api.github.com/users/LamarMoore/events{/privacy}","received_events_url":"https://api.github.com/users/LamarMoore/received_events","type":"User","site_admin":false},"assignees":[{"login":"LamarMoore","id":15360272,"avatar_url":"https://avatars.githubusercontent.com/u/15360272?v=3","gravatar_id":"","url":"https://api.github.com/users/LamarMoore","html_url":"https://github.com/LamarMoore","followers_url":"https://api.github.com/users/LamarMoore/followers","following_url":"https://api.github.com/users/LamarMoore/following{/other_user}","gists_url":"https://api.github.com/users/LamarMoore/gists{/gist_id}","starred_url":"https://api.github.com/users/LamarMoore/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/LamarMoore/subscriptions","organizations_url":"https://api.github.com/users/LamarMoore/orgs","repos_url":"https://api.github.com/users/LamarMoore/repos","events_url":"https://api.github.com/users/LamarMoore/events{/privacy}","received_events_url":"https://api.github.com/users/LamarMoore/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/mantidproject/mantid/milestones/58","html_url":"https://github.com/mantidproject/mantid/milestones/Release%203.8","labels_url":"https://api.github.com/repos/mantidproject/mantid/milestones/58/labels","id":1505614,"number":58,"title":"Release 3.8","description":"","creator":{"login":"granrothge","id":5910574,"avatar_url":"https://avatars.githubusercontent.com/u/5910574?v=3","gravatar_id":"","url":"https://api.github.com/users/granrothge","html_url":"https://github.com/granrothge","followers_url":"https://api.github.com/users/granrothge/followers","following_url":"https://api.github.com/users/granrothge/following{/other_user}","gists_url":"https://api.github.com/users/granrothge/gists{/gist_id}","starred_url":"https://api.github.com/users/granrothge/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/granrothge/subscriptions","organizations_url":"https://api.github.com/users/granrothge/orgs","repos_url":"https://api.github.com/users/granrothge/repos","events_url":"https://api.github.com/users/granrothge/events{/privacy}","received_events_url":"https://api.github.com/users/granrothge/received_events","type":"User","site_admin":false},"open_issues":706,"closed_issues":466,"state":"open","created_at":"2016-01-13T20:00:53Z","updated_at":"2016-08-01T13:12:17Z","due_on":"2016-10-09T23:00:00Z","closed_at":null},"commits_url":"https://api.github.com/repos/mantidproject/mantid/pulls/17049/commits","review_comments_url":"https://api.github.com/repos/mantidproject/mantid/pulls/17049/comments","review_comment_url":"https://api.github.com/repos/mantidproject/mantid/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mantidproject/mantid/issues/17049/comments","statuses_url":"https://api.github.com/repos/mantidproject/mantid/statuses/666bcfe943f957810eb0f402808783f9c9087235","head":{"label":"mantidproject:fix_EventList_performance_regression","ref":"fix_EventList_performance_regression","sha":"666bcfe943f957810eb0f402808783f9c9087235","user":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"repo":{"id":2594116,"name":"mantid","full_name":"mantidproject/mantid","owner":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mantidproject/mantid","description":"Main repository for Mantid code","fork":false,"url":"https://api.github.com/repos/mantidproject/mantid","forks_url":"https://api.github.com/repos/mantidproject/mantid/forks","keys_url":"https://api.github.com/repos/mantidproject/mantid/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mantidproject/mantid/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mantidproject/mantid/teams","hooks_url":"https://api.github.com/repos/mantidproject/mantid/hooks","issue_events_url":"https://api.github.com/repos/mantidproject/mantid/issues/events{/number}","events_url":"https://api.github.com/repos/mantidproject/mantid/events","assignees_url":"https://api.github.com/repos/mantidproject/mantid/assignees{/user}","branches_url":"https://api.github.com/repos/mantidproject/mantid/branches{/branch}","tags_url":"https://api.github.com/repos/mantidproject/mantid/tags","blobs_url":"https://api.github.com/repos/mantidproject/mantid/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mantidproject/mantid/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mantidproject/mantid/git/refs{/sha}","trees_url":"https://api.github.com/repos/mantidproject/mantid/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mantidproject/mantid/statuses/{sha}","languages_url":"https://api.github.com/repos/mantidproject/mantid/languages","stargazers_url":"https://api.github.com/repos/mantidproject/mantid/stargazers","contributors_url":"https://api.github.com/repos/mantidproject/mantid/contributors","subscribers_url":"https://api.github.com/repos/mantidproject/mantid/subscribers","subscription_url":"https://api.github.com/repos/mantidproject/mantid/subscription","commits_url":"https://api.github.com/repos/mantidproject/mantid/commits{/sha}","git_commits_url":"https://api.github.com/repos/mantidproject/mantid/git/commits{/sha}","comments_url":"https://api.github.com/repos/mantidproject/mantid/comments{/number}","issue_comment_url":"https://api.github.com/repos/mantidproject/mantid/issues/comments{/number}","contents_url":"https://api.github.com/repos/mantidproject/mantid/contents/{+path}","compare_url":"https://api.github.com/repos/mantidproject/mantid/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mantidproject/mantid/merges","archive_url":"https://api.github.com/repos/mantidproject/mantid/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mantidproject/mantid/downloads","issues_url":"https://api.github.com/repos/mantidproject/mantid/issues{/number}","pulls_url":"https://api.github.com/repos/mantidproject/mantid/pulls{/number}","milestones_url":"https://api.github.com/repos/mantidproject/mantid/milestones{/number}","notifications_url":"https://api.github.com/repos/mantidproject/mantid/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mantidproject/mantid/labels{/name}","releases_url":"https://api.github.com/repos/mantidproject/mantid/releases{/id}","deployments_url":"https://api.github.com/repos/mantidproject/mantid/deployments","created_at":"2011-10-17T19:07:38Z","updated_at":"2016-07-15T19:42:24Z","pushed_at":"2016-08-01T13:12:17Z","git_url":"git://github.com/mantidproject/mantid.git","ssh_url":"git@github.com:mantidproject/mantid.git","clone_url":"https://github.com/mantidproject/mantid.git","svn_url":"https://github.com/mantidproject/mantid","homepage":"www.mantidproject.org","size":430887,"stargazers_count":46,"watchers_count":46,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":34,"mirror_url":null,"open_issues_count":1630,"forks":34,"open_issues":1630,"watchers":46,"default_branch":"master"}},"base":{"label":"mantidproject:master","ref":"master","sha":"6a3e45f40997de8022656fd816172f8c92cb0515","user":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"repo":{"id":2594116,"name":"mantid","full_name":"mantidproject/mantid","owner":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mantidproject/mantid","description":"Main repository for Mantid code","fork":false,"url":"https://api.github.com/repos/mantidproject/mantid","forks_url":"https://api.github.com/repos/mantidproject/mantid/forks","keys_url":"https://api.github.com/repos/mantidproject/mantid/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mantidproject/mantid/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mantidproject/mantid/teams","hooks_url":"https://api.github.com/repos/mantidproject/mantid/hooks","issue_events_url":"https://api.github.com/repos/mantidproject/mantid/issues/events{/number}","events_url":"https://api.github.com/repos/mantidproject/mantid/events","assignees_url":"https://api.github.com/repos/mantidproject/mantid/assignees{/user}","branches_url":"https://api.github.com/repos/mantidproject/mantid/branches{/branch}","tags_url":"https://api.github.com/repos/mantidproject/mantid/tags","blobs_url":"https://api.github.com/repos/mantidproject/mantid/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mantidproject/mantid/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mantidproject/mantid/git/refs{/sha}","trees_url":"https://api.github.com/repos/mantidproject/mantid/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mantidproject/mantid/statuses/{sha}","languages_url":"https://api.github.com/repos/mantidproject/mantid/languages","stargazers_url":"https://api.github.com/repos/mantidproject/mantid/stargazers","contributors_url":"https://api.github.com/repos/mantidproject/mantid/contributors","subscribers_url":"https://api.github.com/repos/mantidproject/mantid/subscribers","subscription_url":"https://api.github.com/repos/mantidproject/mantid/subscription","commits_url":"https://api.github.com/repos/mantidproject/mantid/commits{/sha}","git_commits_url":"https://api.github.com/repos/mantidproject/mantid/git/commits{/sha}","comments_url":"https://api.github.com/repos/mantidproject/mantid/comments{/number}","issue_comment_url":"https://api.github.com/repos/mantidproject/mantid/issues/comments{/number}","contents_url":"https://api.github.com/repos/mantidproject/mantid/contents/{+path}","compare_url":"https://api.github.com/repos/mantidproject/mantid/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mantidproject/mantid/merges","archive_url":"https://api.github.com/repos/mantidproject/mantid/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mantidproject/mantid/downloads","issues_url":"https://api.github.com/repos/mantidproject/mantid/issues{/number}","pulls_url":"https://api.github.com/repos/mantidproject/mantid/pulls{/number}","milestones_url":"https://api.github.com/repos/mantidproject/mantid/milestones{/number}","notifications_url":"https://api.github.com/repos/mantidproject/mantid/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mantidproject/mantid/labels{/name}","releases_url":"https://api.github.com/repos/mantidproject/mantid/releases{/id}","deployments_url":"https://api.github.com/repos/mantidproject/mantid/deployments","created_at":"2011-10-17T19:07:38Z","updated_at":"2016-07-15T19:42:24Z","pushed_at":"2016-08-01T13:12:17Z","git_url":"git://github.com/mantidproject/mantid.git","ssh_url":"git@github.com:mantidproject/mantid.git","clone_url":"https://github.com/mantidproject/mantid.git","svn_url":"https://github.com/mantidproject/mantid","homepage":"www.mantidproject.org","size":430887,"stargazers_count":46,"watchers_count":46,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":34,"mirror_url":null,"open_issues_count":1630,"forks":34,"open_issues":1630,"watchers":46,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mantidproject/mantid/pulls/17049"},"html":{"href":"https://github.com/mantidproject/mantid/pull/17049"},"issue":{"href":"https://api.github.com/repos/mantidproject/mantid/issues/17049"},"comments":{"href":"https://api.github.com/repos/mantidproject/mantid/issues/17049/comments"},"review_comments":{"href":"https://api.github.com/repos/mantidproject/mantid/pulls/17049/comments"},"review_comment":{"href":"https://api.github.com/repos/mantidproject/mantid/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mantidproject/mantid/pulls/17049/commits"},"statuses":{"href":"https://api.github.com/repos/mantidproject/mantid/statuses/666bcfe943f957810eb0f402808783f9c9087235"}},"merged":true,"mergeable":null,"mergeable_state":"unknown","merged_by":{"login":"FedeMPouzols","id":5257172,"avatar_url":"https://avatars.githubusercontent.com/u/5257172?v=3","gravatar_id":"","url":"https://api.github.com/users/FedeMPouzols","html_url":"https://github.com/FedeMPouzols","followers_url":"https://api.github.com/users/FedeMPouzols/followers","following_url":"https://api.github.com/users/FedeMPouzols/following{/other_user}","gists_url":"https://api.github.com/users/FedeMPouzols/gists{/gist_id}","starred_url":"https://api.github.com/users/FedeMPouzols/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/FedeMPouzols/subscriptions","organizations_url":"https://api.github.com/users/FedeMPouzols/orgs","repos_url":"https://api.github.com/users/FedeMPouzols/repos","events_url":"https://api.github.com/users/FedeMPouzols/events{/privacy}","received_events_url":"https://api.github.com/users/FedeMPouzols/received_events","type":"User","site_admin":false},"comments":3,"review_comments":0,"commits":2,"additions":8,"deletions":8,"changed_files":1}}
{"actor":{"display_login":"FedeMPouzols"}}