PolyhedralDev / Terra

Voxel world generation modding platform
MIT License
638 stars 83 forks source link

Implement image cache strategies to reduce excessive memory consumption #421

Closed TheDarkSword closed 8 months ago

TheDarkSword commented 10 months ago

I'm using Terra and it uses 3GB of heap perpetually. There are 9 instances of "com.dfsek.terra.addons.image.image.BufferedImageWrapper" loaded permanently, each of them has a byte array (byte[]) occupying 400 MB. Isn't there a way to avoid keeping them permanently loaded in memory or to load only the necessary parts when necessary?

Astrashh commented 10 months ago

If you are using config packs that use images, they will stay in memory for use in generation if the config pack is loaded. How unusual your memory usage is would be determined by the contents of your installed packs, so sending those would be helpful in diagnosing if excess memory is being used, or if those packs just use a few larger images.

Only keeping parts of the image in memory would be tricky, and determining which parts would need to be loaded when would be even more difficult as its assumed any part of the image could be used for any newly generated chunks.

It would however be possible to lazily load into the image cache, which would probably be useful for those pre-generating who don't need the images to stay in memory once chunks are generated.

TheDarkSword commented 10 months ago

In my case the world is already generated so there is actually no need to keep them permanently loaded.

Regarding the fact of uploading a part of it, I was thinking of dividing the images into chunks (as if they were Minecraft chunks) and then loading and unloading the image chunks on demand.

I await your feedback on the issue and if necessary I can fork the repo and apply the changes myself to help you with your work.

solonovamax commented 10 months ago

Regarding the fact of uploading a part of it, I was thinking of dividing the images into chunks (as if they were Minecraft chunks) and then loading and unloading the image chunks on demand.

this would be not fun™, because doing IO during chunk gen is, uh, bad lol

but, there may be a way to extract the relevant data from the image (eg. the data we care about), and store that in a way that uses less memory? (or maybe some funny in-memory compression stuff?) unsure.

TheDarkSword commented 10 months ago

Regarding the fact of uploading a part of it, I was thinking of dividing the images into chunks (as if they were Minecraft chunks) and then loading and unloading the image chunks on demand.

this would be not fun™, because doing IO during chunk gen is, uh, bad lol

but, there may be a way to extract the relevant data from the image (eg. the data we care about), and store that in a way that uses less memory? (or maybe some funny in-memory compression stuff?) unsure.

During chunk generation this option must obviously be disabled. I already have all the chunks generated so it's useless to have 3 GB of RAM full of useless information, so I'm looking for a solution for this

solonovamax commented 10 months ago

tbh, it may just be possible to remove terra entirely, if chunks are already generated

duplexsystem commented 10 months ago

No do not do this. This is bad. Terra uses custom biome types. This will (I think permanently) remove them from the world.

Astrashh commented 10 months ago

Regarding the fact of uploading a part of it, I was thinking of dividing the images into chunks (as if they were Minecraft chunks) and then loading and unloading the image chunks on demand.

We have the STITCHED_BITMAP Image config type (docs) which lets you do this minus the loading / unloading. Going to trial lazy loading and possibly caffeine caches rather than a ConcurrentHashMap and see how that goes. Combining that with STITCHED_BITMAP would only load each image 'chunk' when needed, not sure how eviction would go though

Current caching code is done here for reference https://github.com/PolyhedralDev/Terra/blob/72686601ee4d1c24bd508a31a0cf1c5e5751c000/common/addons/library-image/src/main/java/com/dfsek/terra/addons/image/config/image/ImageCache.java

solonovamax commented 10 months ago

No do not do this. This is bad. Terra uses custom biome types. This will (I think permanently) remove them from the world.

that's why I said "may"

afaik if you're on paper, it should be fine, but I could be wrong

Astrashh commented 10 months ago

The server will most likely just set the biome to some default (iirc minecraft:plains) every time a chunk is loaded with unregistered biomes then spam console, at least this is what I have observed in the past with fabric dev env