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

Map memory speedup #27457

Merged
merged 2 commits into from
Jan 13, 2019
Merged

Conversation

kevingranade
Copy link
Member

@kevingranade kevingranade commented Jan 6, 2019

Summary

SUMMARY: Performance "Only memorize map features the first time they are seen"

Purpose of change

Currently map memory consumes approximately 1.5% of the game's overall CPU utilization just memorizing tiles. The reason for this is every turn it re-memorizes every tile in sight (up to 14,641 tiles, not counting other-z-level tiles!), making a not-that-expensive operation problematically slow. Worse, in debug mode it consumes many times this much time because of the use of debug containers. With this cache, this full-screen memorization can only occur upon changing z-levels, and thereafter can only trigger a number of memorizations equal to the new tiles seen in a turn.

Describe the solution

Added a cache of which tiles have been seen since their most recent load, and only memorize the tile if it has not been seen before.
Initial perf testing indicates that in release builds the overhead from map memorization has become immeasurably small. The new caching logic likewise does not appear in profiling.

Describe alternatives you've considered

Reverse tile memorization to only occur when tiles move out of sight instead of when they first become visible. This is more correct, but the caching logic to do this is much more complicated and potentially expensive than the approach in this PR.

TODO

  • Use cache in tiles mode.
  • Verify speedup in debug builds.
  • Add cache invalidation logic to map::set_ter() and map::set_furn().
  • Investigate a possible off-by-one error, initial testing while walking back and forth showed strange bars of un-memorized terrain.

@mlangsdorf mlangsdorf added Code: Performance Performance boosting code (CPU, memory, etc.) [C++] Changes (can be) made in C++. Previously named `Code` Mechanics: Map Memory Performance issues, weird behavior, improvements to map memory feature labels Jan 6, 2019
@kevingranade kevingranade force-pushed the map-memory-speedup branch 3 times, most recently from 95358c4 to dc85994 Compare January 7, 2019 00:14
@kevingranade
Copy link
Member Author

Tested in a debug build. Hoooooooleeeeeeeshiiiiittttt it's slow without this.
I think I'll have it licked once I sort out the remaining issues.

@kevingranade kevingranade force-pushed the map-memory-speedup branch 2 times, most recently from 6322aae to 7e53984 Compare January 7, 2019 07:48
@kevingranade
Copy link
Member Author

Nominally done now, though haven't tested the tiles support yet.
A few potentially fragile things to check:

  • Walking back and forth exercises memory a bit.
  • Damaging vehicles while in sight, then check that the damage is reflected in memory.
  • Damaging terrain and furniture and doing the above.
  • Ramming vehicles into things (it's just fun anyway).
  • Blowing shit up (same).

@Epictyphlosion
Copy link
Contributor

If this helps take some weight off of my and others' CPU's while playing, then I this should be added ASAP!

src/map.cpp Show resolved Hide resolved
src/map.h Outdated Show resolved Hide resolved
src/cata_tiles.cpp Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) Mechanics: Map Memory Performance issues, weird behavior, improvements to map memory feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants