{"action":"created","issue":{"url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/issues/26933","repository_url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA","labels_url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/issues/26933/labels{/name}","comments_url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/issues/26933/comments","events_url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/issues/26933/events","html_url":"https://github.com/CleverRaven/Cataclysm-DDA/pull/26933","id":387054139,"node_id":"MDExOlB1bGxSZXF1ZXN0MjM1NjMwMTQx","number":26933,"title":"inventory::invlet_cache overhaul","user":{"login":"jbytheway","id":52664,"node_id":"MDQ6VXNlcjUyNjY0","avatar_url":"https://avatars3.githubusercontent.com/u/52664?v=4","gravatar_id":"","url":"https://api.github.com/users/jbytheway","html_url":"https://github.com/jbytheway","followers_url":"https://api.github.com/users/jbytheway/followers","following_url":"https://api.github.com/users/jbytheway/following{/other_user}","gists_url":"https://api.github.com/users/jbytheway/gists{/gist_id}","starred_url":"https://api.github.com/users/jbytheway/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jbytheway/subscriptions","organizations_url":"https://api.github.com/users/jbytheway/orgs","repos_url":"https://api.github.com/users/jbytheway/repos","events_url":"https://api.github.com/users/jbytheway/events{/privacy}","received_events_url":"https://api.github.com/users/jbytheway/received_events","type":"User","site_admin":false},"labels":[{"id":887839346,"node_id":"MDU6TGFiZWw4ODc4MzkzNDY=","url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/labels/Code:%20Astyle%20/%20Optimization","name":"Code: Astyle / Optimization","color":"cc126f","default":false},{"id":40575029,"node_id":"MDU6TGFiZWw0MDU3NTAyOQ==","url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/labels/Code:%20Performance","name":"Code: Performance","color":"53e719","default":false},{"id":620181519,"node_id":"MDU6TGFiZWw2MjAxODE1MTk=","url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/labels/Inventory%20/%20AIM","name":"Inventory / AIM","color":"FF8000","default":false},{"id":659366627,"node_id":"MDU6TGFiZWw2NTkzNjY2Mjc=","url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/labels/[C++]","name":"[C++]","color":"f9d0c4","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-12-03T23:38:08Z","updated_at":"2018-12-04T22:22:53Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/pulls/26933","html_url":"https://github.com/CleverRaven/Cataclysm-DDA/pull/26933","diff_url":"https://github.com/CleverRaven/Cataclysm-DDA/pull/26933.diff","patch_url":"https://github.com/CleverRaven/Cataclysm-DDA/pull/26933.patch"},"body":"#### Summary\r\nSUMMARY: Performance \"Improved inventory performance\"\r\n\r\n#### Purpose of change\r\nSome of the test code spends a lot of time updating invlets.  I think we can improve both the performance and the code clarity.\r\n\r\n#### Describe the solution\r\nUpdate invlet_cache to be a bidirectional map -- mapping from ids to invlets but also from invlets to ids.  This removes the need for iterating over the whole invlet cache on several operations.\r\n\r\nThis cuts the runtime of the `starting_items` test by nearly a whole minute on Linux debug builds, and should help on release builds too.\r\n\r\n#### Describe alternatives you've considered\r\nWhile working on this I discovered that the invlet_cache was gradually accumulating empty lists that served no purpose except to slow it down.  We could just prevent that, and that would help somewhat with performance in real games.  But that probably wouldn't really affect the test performance (since the inventories there are short-lived), and I think this solution is better anyway.\r\n\r\nIn any case, that's also fixed by this change.  Empty invlet lists are tidied up at game load time.\r\n\r\n#### Additional context\r\nThis is the first of a number of PRs I'm working on to improve the performance of the tests on Linux debug builds.\r\n\r\nI should perhaps have written tests for this new class, but I'm hoping the existing invlet tests have it covered; there's quite a lot of them.\r\nEdit: I have added some tests after all."},"comment":{"url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/issues/comments/444282089","html_url":"https://github.com/CleverRaven/Cataclysm-DDA/pull/26933#issuecomment-444282089","issue_url":"https://api.github.com/repos/CleverRaven/Cataclysm-DDA/issues/26933","id":444282089,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NDI4MjA4OQ==","user":{"login":"jbytheway","id":52664,"node_id":"MDQ6VXNlcjUyNjY0","avatar_url":"https://avatars3.githubusercontent.com/u/52664?v=4","gravatar_id":"","url":"https://api.github.com/users/jbytheway","html_url":"https://github.com/jbytheway","followers_url":"https://api.github.com/users/jbytheway/followers","following_url":"https://api.github.com/users/jbytheway/following{/other_user}","gists_url":"https://api.github.com/users/jbytheway/gists{/gist_id}","starred_url":"https://api.github.com/users/jbytheway/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jbytheway/subscriptions","organizations_url":"https://api.github.com/users/jbytheway/orgs","repos_url":"https://api.github.com/users/jbytheway/repos","events_url":"https://api.github.com/users/jbytheway/events{/privacy}","received_events_url":"https://api.github.com/users/jbytheway/received_events","type":"User","site_admin":false},"created_at":"2018-12-04T22:22:52Z","updated_at":"2018-12-04T22:22:52Z","author_association":"CONTRIBUTOR","body":"Test failure every time is a mysterious timeout in the clang-6 tests.  I'm hoping this is the same issue as #26941 (but to be honest I doubt it is because that bug appears to be very compiler-codegen specific and this is a totally different compiler).  Either way, I'm happy for this PR to wait until that issue is resolved and we can see where we go from there."}}
{"actor":{"display_login":"jbytheway"}}