BannerlordCE / CEEvents

Captivity Events Source Code
https://www.nexusmods.com/mountandblade2bannerlord/mods/1226
MIT License
4 stars 6 forks source link

Each image present in module subfolder increases memory usage by 1MB, even when unused #79

Closed bicobus closed 1 year ago

bicobus commented 1 year ago

I am making dangerous assumption in the following description of the problem. However, I am fairly confident for them to be accurate.

I've been having an issue with memory bloat. After a bunch of days of investigation I can definitively say that it is linked with the amount of image present in each CEEvents submodules. The image themselves don't matter, could be a single pixel and it wouldn't change anything. The issue at core lies with how the mod loads the images, and how it is stored within memory.

The following is how images are currently loaded: list them all, then assign them to a texture, then store that texture into persistence.

https://github.com/BannerlordCE/CEEvents/blob/57b44716f9790cdb263b032ea0925bf18b00ef03/CESubModule.cs#L327-L360

Issue happens with how heavy the texture object is. To know that, we need to use the command profiler.generate_performance_report. That will generate a bunch of files under Mount & Blade II Bannerlord/bin/Win64_Shipping_Client/profiling_results/. The one we want to take a look at is gpu_memory.txt — btw, this is a report done while in the start menu of the game. In it, you'll see a section named Gpu data textures memory usages., which presumably highlight the issue. Memory consumption rises relative to the texture resolution:

Which means that with a submodule such as Hot Buttered Girls that has more than 2000 images, the space used by the simple listing of image will be over 2 GiB.

Possible resolution

Do not create a texture object right away, instead store the list of files as string. Create the texture when a module request the image, then discard the object once unneeded. The cost of creating the texture object per request is minimal given the context these textures are used. There's literally no intensive work dependent on their existence.

bicobus commented 1 year ago

A way to keep a limited cache is to implement the least recently used cache policy. Keep the last 10 pictures in memory, if a new one appear, override the oldest element. Caveat: animations sequences should be exempt from the cache, as they tends to have a greater amount of images.

A LRU will help during normal gameplay (vanilla game), where often time the same image is displayed over and over again.

TheBadListener commented 1 year ago

@bicobus Thank you for your report, should be solved now! I hope you report any other problems or any new features you will like to see implemented!

bicobus commented 1 year ago

I'll duplicate what I wrote on the discord.

https://github.com/BannerlordCE/CEEvents/blob/c82c0e13bd9c6d3e1d68212546633c5de700a6c7/CESubModule.cs#L202-L207

Dictionary aren't ordered, so asking for the first might not return the oldest element. You may want to used a linked list instead. Linked list store the key as a str, you always AddFirst for new images.

To then trim the persistent storage: get the last entry from the list then remove it RemoveLast. Then remove the element from the dictionary with the retrieved key.

See: https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.linkedlist-1?view=net-7.0

Using python, we call that a deque. They can have a maximum length, which would be useful in this use case.