{"action":"created","issue":{"url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/issues/103","repository_url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod","labels_url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/issues/103/labels{/name}","comments_url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/issues/103/comments","events_url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/issues/103/events","html_url":"https://github.com/We-the-People-civ4col-mod/Mod/issues/103","id":362567580,"node_id":"MDU6SXNzdWUzNjI1Njc1ODA=","number":103,"title":"Cities fails to use CPU cache when looping plots","user":{"login":"Nightinggale","id":4641647,"node_id":"MDQ6VXNlcjQ2NDE2NDc=","avatar_url":"https://avatars0.githubusercontent.com/u/4641647?v=4","gravatar_id":"","url":"https://api.github.com/users/Nightinggale","html_url":"https://github.com/Nightinggale","followers_url":"https://api.github.com/users/Nightinggale/followers","following_url":"https://api.github.com/users/Nightinggale/following{/other_user}","gists_url":"https://api.github.com/users/Nightinggale/gists{/gist_id}","starred_url":"https://api.github.com/users/Nightinggale/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Nightinggale/subscriptions","organizations_url":"https://api.github.com/users/Nightinggale/orgs","repos_url":"https://api.github.com/users/Nightinggale/repos","events_url":"https://api.github.com/users/Nightinggale/events{/privacy}","received_events_url":"https://api.github.com/users/Nightinggale/received_events","type":"User","site_admin":false},"labels":[{"id":1060454076,"node_id":"MDU6TGFiZWwxMDYwNDU0MDc2","url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/labels/Code%20Efficiency","name":"Code Efficiency","color":"daff38","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/milestones/4","html_url":"https://github.com/We-the-People-civ4col-mod/Mod/milestone/4","labels_url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/milestones/4/labels","id":3663784,"node_id":"MDk6TWlsZXN0b25lMzY2Mzc4NA==","number":4,"title":"IDEA","description":"This milestone is for \"feature requests\" that have not yet been consolidated/ upvoted for a milestone.","creator":{"login":"abmpicoli","id":1733170,"node_id":"MDQ6VXNlcjE3MzMxNzA=","avatar_url":"https://avatars0.githubusercontent.com/u/1733170?v=4","gravatar_id":"","url":"https://api.github.com/users/abmpicoli","html_url":"https://github.com/abmpicoli","followers_url":"https://api.github.com/users/abmpicoli/followers","following_url":"https://api.github.com/users/abmpicoli/following{/other_user}","gists_url":"https://api.github.com/users/abmpicoli/gists{/gist_id}","starred_url":"https://api.github.com/users/abmpicoli/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abmpicoli/subscriptions","organizations_url":"https://api.github.com/users/abmpicoli/orgs","repos_url":"https://api.github.com/users/abmpicoli/repos","events_url":"https://api.github.com/users/abmpicoli/events{/privacy}","received_events_url":"https://api.github.com/users/abmpicoli/received_events","type":"User","site_admin":false},"open_issues":42,"closed_issues":0,"state":"open","created_at":"2018-09-18T01:14:11Z","updated_at":"2018-09-21T14:38:46Z","due_on":null,"closed_at":null},"comments":1,"created_at":"2018-09-21T11:10:14Z","updated_at":"2018-09-21T15:14:54Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"When CvCity loops the plots within range, it does so in a spiral, meaning it goes from closest to furthest away. This could make sense for looping, which stops looping when the first usable candidate is found. However that's not what happens. Instead it goes through every single plot within range.\r\n\r\nThe problem with this approach is that CvMap stores the plots linear in memory, meaning they appear like the beam on a CRT monitor. This means looping plots should be done:\r\n\r\n```cpp\r\nfor (y=top to bottom)\r\n    for (x=left to right)\r\n```\r\n\r\nThe actual implementation could be different, but if this read order is used, the CPU cache will be more likely to contain the next read plot, hence reducing memory I/O latency.\r\n\r\nMost likely this affects all code, which loops 3x3 and 5x5 plots. We should investigate how widespread this issue is, but most likely cities are the worst in terms of performance. The AI loops colony plots a lot of times to figure out which professions each unit should have."},"comment":{"url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/issues/comments/423568652","html_url":"https://github.com/We-the-People-civ4col-mod/Mod/issues/103#issuecomment-423568652","issue_url":"https://api.github.com/repos/We-the-People-civ4col-mod/Mod/issues/103","id":423568652,"node_id":"MDEyOklzc3VlQ29tbWVudDQyMzU2ODY1Mg==","user":{"login":"Nightinggale","id":4641647,"node_id":"MDQ6VXNlcjQ2NDE2NDc=","avatar_url":"https://avatars0.githubusercontent.com/u/4641647?v=4","gravatar_id":"","url":"https://api.github.com/users/Nightinggale","html_url":"https://github.com/Nightinggale","followers_url":"https://api.github.com/users/Nightinggale/followers","following_url":"https://api.github.com/users/Nightinggale/following{/other_user}","gists_url":"https://api.github.com/users/Nightinggale/gists{/gist_id}","starred_url":"https://api.github.com/users/Nightinggale/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Nightinggale/subscriptions","organizations_url":"https://api.github.com/users/Nightinggale/orgs","repos_url":"https://api.github.com/users/Nightinggale/repos","events_url":"https://api.github.com/users/Nightinggale/events{/privacy}","received_events_url":"https://api.github.com/users/Nightinggale/received_events","type":"User","site_admin":false},"created_at":"2018-09-21T15:14:54Z","updated_at":"2018-09-21T15:14:54Z","author_association":"CONTRIBUTOR","body":"There are two parts of the code. One is loop order, the other is AI priority. I agree that the AI priority should stay the same and not start to favor the far north-western corner.\r\n\r\nThe vanilla approach is to store int arrays in CvGlobals and then loop the arrays to follow predefined plot order (see below). Those arrays are used by the functions plotCity, plotCityXY(2 overloaded functions) in CvGameCoreUtils.cpp.\r\n\r\nOne example of plot looping is CvCityAI::AI_buildingValue(). It loops all plots to ask isWater() to each when figuring out how useful it will be to build docks. There are plenty of such examples if you start to look for them.\r\n\r\nImproving regarding memory I/O is good, but we should also consider which coding strategy we would want to use here. For instance should CvCity hold an array of plot pointers instead of looking all over the code/memory to figure out how to loop.\r\n\r\n(part of) the code in question from CvGlobals::init()\r\n```cpp\r\n\tint aiCityPlotPriority[NUM_CITY_PLOTS] =\r\n\t{\r\n\t\t0,\r\n\t\t1, 2, 1, 2, 1, 2, 1, 2,\r\n#ifndef ONE_PLOT_CITY_RADIUS\r\n\t\t3, 4, 4, 3, 4, 4, 3, 4, 4, 3, 4, 4,\t// R&R, ray, 2 Plot Radius\r\n\t\t4, 4, 4, 4, // R&R, ray, 2 Plot Radius\r\n#endif\r\n\t};\r\n\r\n\tint aaiXYCityPlot[CITY_PLOTS_DIAMETER][CITY_PLOTS_DIAMETER] =\r\n\t{\r\n#ifdef ONE_PLOT_CITY_RADIUS\r\n\t\t{6, 7, 8,},\r\n\t\t{5, 0, 1,},\r\n\t\t{4, 3, 2,},\r\n#else\r\n\t\t{23, 17, 18, 19, 24,},\t// R&R, ray, 2 Plot Radius - START\r\n\t\t{16, 6, 7, 8, 20,},\r\n\t\t{15, 5, 0, 1, 9,},\r\n\t\t{14, 4, 3, 2, 10,},\r\n\t\t{22, 13, 12, 11, 21,}\t// R&R, ray, 2 Plot Radius - END\r\n#endif\r\n\t};\r\n```"}}
{"actor":{"display_login":"Nightinggale"}}