-
Notifications
You must be signed in to change notification settings - Fork 279
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
Refactor map memory, remove map memory limit. #247
Conversation
Note: this naive implementation significantly lowers FPS
Move memorized submaps that would be affected during drawing into a 2d array.
Looks really good. |
In the future, I'll want to have it that way, but that's distant future. Shrinking it to
I like 3 the best for now, since file access is much slower on Windows than it is on Linux, so I couldn't test the performance without booting Windows up. |
I didn't want
Yes, that's why I was considering more of a hybrid approach. The game right now generates terrain for some submaps from scratch, and theoretically it would be possible to have But all of that sounds not very robust...
Speaking of, map memory has weird behavior regarding 3d vision. I've updated PR description. |
Sure. Your code doesn't lock the idea out or anything, and it's easier to comprehend and optimize than having to deal with full submaps.
That is, memory of a tile would be a full copy of said tile, made at the time of memorization.
If it was loaded only on a special
Consider:
Now, it requires arcane stuff to happen and is very minor, so it totally wouldn't be considered a blocker or anything. Just an edge case that can't be solved perfectly without something like an observer pattern that updates all |
That could be expensive memory wise. Would also require some clever tricks to keep the rendering consistent, since some tiles, like walls or furniture, are rendered based on their connection to neighbours.
Yeah, that's what I was trying to say. I'm not the best at explanations, heh. |
Yes, up to double. But it would allow memorizing items and vehicles, which would be incredibly convenient if you wanted to say, find the nearest pair of welding goggles, vehicle with V12, check up on your stash to see if you're missing something etc. It might actually decrease memory usage too: storing entire tile strings could get expensive.
Doesn't sound too hard. Say, for every category of tile connection, generate an invisible, fake variant. It would never be drawn, but it would be memorized next to tiles that connect to things, if the real neighbor wasn't memorized. |
From what I've read across the internet, memory cost from storing small strings in C++ depends heavily on compiler and which standard library it uses. I don't expect worst-case Then there's the fact that other z-levels are unlikely to have But I'm curious about the stress test myself - will see what can be done. |
Okay, I've made a few changes to the code based on the review, shortened Results for a fresh test case (grabbed a car and drove through fields and a small town; 115'568 tiles memorized ):
Looks good to me. TODO:
|
Ideally, all of the functions in the interface would use the same scale. |
Collect map::draw code in one place, simplify indexing/bound checks. Remove the "batch drawing" optimization: it doesn't help with drawing, but harms code clarity. Instead, convert all relevant draw methods to use 'wputch' and rely on caller to position the cursor properly.
Here's a bunch of memory tests (I love tests); all "memory usage" was measured as heap usage. From compiler's PoV, Simulating "empty" tile memory. Simulating "full" tile memory. Practical test 1. Memory usage, kb, old implementation vs the shiny new one:
Apparently, memory usage in this case is almost identical. I've tried this 2 times, and the numbers are within ±100 kb. Practical test 2.
And this gives ≈7.88 kb per Conclusion:
Besides that, I feel this PR is ready. I don't see any wonky behavior with tile memory, and the map drawing seems correct (had to tweak it a bit to allow memorizing off-screen tiles in ascii mode). |
Looks good, didn't see any problems during testing. Ideally, memorizing tiles wouldn't require actually looking up and down z-levels manually. Though this looks like it could be a real performance hog, since from what I can tell, at the moment memorizing a tile requires actually drawing it. |
After glancing over how cataclysm's save data is stored, I'm wondering: why is it not compressed in any way? All the .map json files that take up the most space have remarkably compressible structure and just compressing a bunch of them with default zip compression resulted in ~30x compression ratio which is only possible because of repetitive nature of their contents. So is it possible to use something like zlib to do transparent compression/decompression for all in-game file operations? It should be pretty simple to convert all text files to be compressed and uncompressed completely transparently for the rest of the code. There's lots of libraries that do that, for example https://github.com/rikyoz/bit7z or just zlib. This could easily reduce overall memory usage and increase file access speed by a factor of 10 to 50. Besides, windows file access is slow. Reading even compressed files from a single pack is way faster than reading individual uncompressed files. |
Yes, CleverRaven#44218. I've run some tests using the code from there - compressing each While it is good, I see this as waste of potential. It also has a downside - compressing a save full of compressed But that's how that particular solution works, other approaches may work better.
It could work, yes. If that library (or some other potential similar library) supports modifying archives, it should be easy to rig a system that would stuff the The question is how that would impact save/load time (I see it going both ways), and how well that library supports other platforms (we're speaking Linux/Windows/Mac/Android, x86/x64). |
Obviously next step is to use some sort of a virtual file system instead of actual file system to store everything in one file. Compressing a bunch of files (especially similar ones) is always superior to compressing each file individually. Bundling files using packaging like this is how they optimize slow HDD access on consoles, added benefit of also drastically compressing content in case of Cataclysm comes as just a bonus. Heck, this is how quake engine has always worked, if I remember correctly. So even counting in all the compression/decompression overhead, I still expect to see a significant improvement in terms of both performance and memory. We use our own implementation of such file system in our engine (Path of Exile), but here's a bunch OSS examples of such libraries: https://github.com/yevgeniy-logachev/vfspp , https://github.com/anthony-y/tiny-vfs . Note that they're typically designed to be a drop-in replacement for a physical filesystem which's pretty convenient. |
7z/xz/LZMA is notoriously slow to compress. Cataclysm saves are written about as often as they are read, so compression speed does matter. |
A trick that should be possible to do is to run index once (say, in build time) to create the dictionary that will be used for future compression so that the compressor would not have to rebuild it every time it needs to recompress something. This can work, because I expect the dictionary to change very little as data structure is very similar for different playthroughs and users. I'm not sure which exact libraries support this feature, but might be handy to look out for something like this. |
This is explicitly supported on |
Another random fact: Cataclysm should not need nearly as much write bandwidth as it needs read bandwidth, because in theory during playtime it only needs to save/compress changes (which's not much), but whenever you load a game, it needs to load/decompress the entire world. So (again, in theory, I'm not sure how saves are actually implemented), compression time should not be nearly as important as decompression time. |
It needs to touch all the timestamps for all loaded submaps. In theory, for uncompressed submaps, it would be possible to update only a few bits, but it doesn't work for compressed ones. At least not without extracting timestamps out of them. Debug world, 23 mb
Krwak's world, 151 mb
TL;DR: |
I think zstd looks like a good backend, but I think from implementation and maintenance standpoint it's more reasonable to pick a library that already does all the vfs packing/unpacking/updating transparently even if it'd have slightly worse performance parameters. Or maybe it'd just use zstd as a backend, idk. I was always on the fence of using C-style unsafe libraries in C++ projects without a proper safe wrapper with smart pointers and stuff, but this one is up to you guys to decide, I have no clue what guidelines you follow in this repo. |
Summary
SUMMARY: Features "Unlimited map memory"
Purpose of change
Remove limit on map memory capacity.
Follows #97.
Describe the solution
Right now, map memory is implemented via
lru_cache
, with absolute map square coordinates as keys. This makes it impossible to remove limit on map memory size, since sooner or later players' computers would run out of memory trying to cram all memorized tiles into RAM.This PR splits map memory into chunks (named
memorized_submap
, with size equal to an ordinarysubmap
), and loads these chunks from the disk (or allocates new empty ones) as necessary.The
memorized_submap
s are stored in a new folder,<save-name>/<player-name>.mm1/
, in individual files with names equal to their coordinates. While simplistic, this solution produces a ton of small (<4kb) files, which could tank performance on some systems (e.g. Windows with an antivirus).For ease of testing, I've implemented migration from existing memory map file
<save-name>/<player-name>.mm
on first load.Upd: changed saving to use regions of 8x8 submaps to cut down on number of files.
Describe alternatives you've considered
Mostly tweaking some numbers, e.g. size of
memorized_submap
, but I'd like to keep it equal to the size of asubmap
to avoid unnecessary coordinate conversions.Also, there are some thoughts on how to deal with high data fragmentation on disk, I'll go into details below.
Testing
FPS
I've compiled the game in Visual Studio in 'Release' mode and ran in-game
Draw benchmark
while measuring it with Visual Studio's profiler.For testing purposes, I've selected the following case: a somewhat small map memory size of 71'843 tiles (normal map memory limit in DDA is 57'600, with memory banks bionic it's 2'880'000; default limit in BN right now is 576'000), zoomed-out terrain with no visible tiles, but approx. half of them have been memorized (see attached screenshot).
According to the profiler, time spent inside
avatar::get_memorized_tile()
went down from 10.43% to 0.11%, and the similar thing has likely happened toavatar::memorize_tile()
, but that one is a bit more problematic to measure since it's already optimized bymemory_seen_cache
.Overall, this is a performance improvement, a one that wasn't even ported from DDA (yay!).
Disk usage
In that test case, the size of the existing tile memory file was 2'089'403 bytes, and after migration the total size of all saved
memorized_submap
s went down to 1'535'971 bytes. Likely because old tile memory had to save data as array ofglobal map square coord + memorized_tile
, whilememorized_submap
s are saved as arrays ofmemorized_tile
s.Disk usage, on the other hand, went up from 2'093'056 to 2'359'296 bytes due to huge number of files (1 vs 570).
Another test case. I've created a new character, and played for a few IRL hours.
In the end,
maps
folder had 1'115 files with total size of 7'442'102 bytes (9'289'728 on disk), and the tile memory folder had 3'083 files with total size of 9'070'291 bytes (12'730'368 on disk), with eachmemorized_submap
taking from 1'591 to 4'033 bytes.This makes me think that some sort of compression would be nice to have. Current ideas:
submap
s already do that with tiles and furniture, and it would help withmemorized_submap
s that are mostly empty or contain many similar tiles (e.g. rivers), but would likely have no effect on all othermemorized_submap
s since for each physical tile there are many variations of graphical tile (e.g.t_wall
vst_wall
with connections to neighboring walls).memorized_submap
to an array ofbool
eans, but I'm afraid it would require much work for little gain.memorized_submap
s in bundles.mapbuffer
already does that withsubmap
s, writing 4 of them (2 by 2) per file. Sincememorized_submap
s don't eat as much RAM, they could be packed in bigger chunks (e.g. 6x6 or 8x8). This would cut down on disk usage, and improve save/load performance.Gzip
compression. As CleverRaven#44218 demonstrated, it shouldn't be that hard, and the gains are huge.I'm currently leaning towards 1+3, with possible 4 in the future.
Upd: implemented 1 and 3, results can be seen here: #247 (comment)
Quirks
Existing map memory code has some quirks. Mainly, what is memorized or not depends on currently displayed terrain. Even with 3d vision enabled, to memorize above ground terrain you have to press
x
and look at the other levels. I'm not sure if that needs fixing (probably yes), and how fixing that would impact performance / disk usage.Additionally, in ascii mode the game does not commit to memory tiles that are not visible on screen. This one is probably easy to fix, and I'll most likely include the fix in this PR.
Upd: fixed the ascii mode bug. It required a bit of drawing code cleanup - the cleanup is pure refactor and should not impact the drawing in any way (unless I botched it up, ofc).
Screenshots
Testing area