Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimized map::generate a bit #75188

Merged
merged 2 commits into from
Jul 26, 2024
Merged

Conversation

PatrikLundell
Copy link
Contributor

@PatrikLundell PatrikLundell commented Jul 23, 2024

Summary

None

Purpose of change

Improve performance of map::generate. This was triggered by #75166, but I don't consider this to solve that report.

Describe the solution

Replace repeated calls to MAPBUFFER.lookup_submap to see if it exists with a single check to a new submap_exists operation and then store the results in a vector that's then used for the subsequent existence checks.

Describe alternatives you've considered

  • Waiting for more expert opinions on what to improve.
  • Trying to see if anything can be done about map::is_main_cleanup_queued as that operation seems to use 5.28% of the CPU used in the last testing run (for the same reason lookup_submap costs a lot, i.e. a large number of calls). Out of scope for this PR.
  • Restructure map::loadn code and usage to take the 3D nature of maps into account by loading vertical stacks of maps rather than one level at a time (when the maps support Z levels, of course), and possible also the fact that you don't load a single submap stack, but 4, 11, 21, or a full map's worth, and you ought to be able to gain some synergy by loading all of the stacks from a generated/loaded omt without reevaluating whether you need to generate/load it for every stack. Note such a change doesn't really relate to the recently introduced mapgen 3D support, as maps (and thus their loading) have been 3D for a long time before that change. Out of scope for this PR.
    Edit: Tried it and didn't it to work properly (light isn't set properly), but nevertheless, a performance test showed no significant performance change compared to the master code, presumably because juggling vectors to store state gobbles up the gains made elsewhere. Abandoning this as a failure.

Testing

Bumbling running of the VS profiler before and after modifying the code.
The results seemed to indicate a drop of the overall CPU consumption of map::generate from 15.17% of the CPU budget before the change to 5.28% after. An intermediate run after the changes to generate but before the addition to mapbuffer gave a result that was somewhere in between these two values.
Running the game by teleporting straight to the east to the edge of the revealed map (guaranteeing it hasn't been generated), starting recording and then keep moving east by keeping the movement key pressed until safe mode reported a monster showed it still hesitates at times (and I think it's probably on OMT boundaries). I can't say if the effect is smaller. I'd probably need a slower computer for that.

Additional context

It's quite possible the variability in results from different runs is high enough to make single measurements rather pointless.

@github-actions github-actions bot added Map / Mapgen Overmap, Mapgen, Map extras, Map display [C++] Changes (can be) made in C++. Previously named `Code` astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jul 23, 2024
@Maleclypse Maleclypse merged commit 3b1ad6e into CleverRaven:master Jul 26, 2024
28 of 33 checks passed
@PatrikLundell PatrikLundell deleted the generate branch July 26, 2024 07:17
@Terrorforge Terrorforge mentioned this pull request Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants