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

Restrict total number of chunk update threads based off heap size #1918

Closed

Conversation

embeddedt
Copy link
Contributor

@embeddedt embeddedt commented Jul 28, 2023

Proposed Changes

With the right combination of optimization mods and configs it is possible to run the game on very small heap sizes (<=256MB). However, due to Sodium allowing users to configure the number of chunk builder threads as high as they would like, it is possible to starve the game of memory by having too many threads attempting to update chunks at once.

This PR solves that issue by introducing additional logic that caps the number of threads based off maximum heap size. Vanilla's chunk builder uses its own heuristic dependent on the RenderLayers' expected buffer sizes. Here, we use simpler logic, and allow one thread for every 64MB of heap. On the default vanilla RAM allocation (2GB) this would permit 32 threads, which should still be plenty (Sodium doesn't even try to use more than 10 by default).

Example screenshot attached showing that the maximum number of threads is now 2 even on a 4-thread system due to the low heap size.

image

@HyperSoop
Copy link

I don't think this is needed, it's not a system any significant amount of users would encounter and if you're trying to run the game on heap sizes low enough to make it choke on higher thread counts (and therefore, for this logic to take effect) it's likely you know what you're doing anyway.

@douira
Copy link
Collaborator

douira commented Jul 29, 2023

lgtm

Use a conservative metric of 64MB per update thread
@embeddedt embeddedt force-pushed the add/worker-thread-cap branch from 2174596 to e830899 Compare August 4, 2023 13:59
@embeddedt
Copy link
Contributor Author

if you're trying to run the game on heap sizes low enough to make it choke on higher thread counts (and therefore, for this logic to take effect) it's likely you know what you're doing anyway.

IMO it's bad practice for a mod to crash in an impossible-to-satisfy configuration, especially if that option is exposed in a way that's easy for a user to modify, and if it can easily be prevented with a small code change (as done here). It leaves a bad impression when a mod crashes even if it could be argued the user was at fault for using it wrong. I can also say from anecdotal experience that at least one person I know has experienced crashes and bad performance as a result of setting this value too high for the given heap size.

It's not going to be a perfect solution for modded (the heap could be large there but also be very full) but it's at least possible to more gracefully handle this solution for vanilla gameplay.

@embeddedt embeddedt deleted the add/worker-thread-cap branch February 18, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants