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

feat: make number of chunk processing threads configurable #5237

Merged
merged 3 commits into from
Apr 28, 2024

Conversation

BenjaminAmos
Copy link
Contributor

Contains

This pull request adds a setting to control the maximum number of available worker threads for chunk processing. More precisely, it controls the number of pooled threads available to ChunkProcessingPipeline.

From testing, I have found that reducing the number of chunk threads can noticeably reduce overall CPU utilisation. Although modern operating systems are very good at multitasking, I have noticed detrimental system-wide effects when the game occupies too much processor time at once. The ideal quantity will vary from player to player and so I think this may be a useful option to have configurable.

How to test

  • From the main menu, enter the Video Settings screen via Settings->Video.
  • Scroll down until you see the Chunk Threads setting on the left.
  • Move the slider around to different values. When the slider is 0, it should say Auto.
  • For each possible slider value, start a new game. You should still see chunks being generated around you. With fewer chunk threads new chunks may appear more slowly but the overall CPU utilisation should be lower (this may vary depending on your machine).
  • Quit the game and re-open it. Verify that the setting you chose before is still visible on the Settings->Video screen.

@github-actions github-actions bot added the Type: Improvement Request for or addition/enhancement of a feature label Apr 22, 2024
chunkThreads.setRange(Runtime.getRuntime().availableProcessors());
chunkThreads.setLabelFunction(input -> {
if (input == 0) {
return "Auto";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this string be translated? I am not sure about how dynamic values are handled at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, that seems very international.

@@ -51,7 +51,7 @@ class ChunkProcessingPipelineTest extends TerasologyTestingEnvironment {

@Test
void simpleProcessingSuccess() throws ExecutionException, InterruptedException, TimeoutException {
pipeline = new ChunkProcessingPipeline((p) -> null, (o1, o2) -> 0);
pipeline = new ChunkProcessingPipeline(0, (p) -> null, (o1, o2) -> 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A value of 0 to represent "Automatic" does seem a bit unexpected in hindsight, Do you think another value would make sense? Or maybe wrapping it in a constant would be enough?

@soloturn soloturn force-pushed the feature/configurable-chunk-threads branch from 7b7fc6d to 5e6b3c1 Compare April 26, 2024 20:23
@BenjaminAmos BenjaminAmos marked this pull request as draft April 27, 2024 16:05
@BenjaminAmos BenjaminAmos marked this pull request as ready for review April 27, 2024 17:04
@soloturn soloturn merged commit 33dabf5 into develop Apr 28, 2024
10 checks passed
@BenjaminAmos BenjaminAmos deleted the feature/configurable-chunk-threads branch June 23, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement Request for or addition/enhancement of a feature
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants