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

Fix horrible lag in dungeons #501

Closed
wants to merge 1 commit into from
Closed

Conversation

TheDGOfficial
Copy link

While loop without a delay starves CPU time in background inside the coroutine's DefaultDispatcher thread, competing with Client thread and causing horrible lag inside dungeons on machines with low amount of CPU cores (<=4)

Add a delay(50) to fix it.


While mainly affecting people with low amount of CPU cores, it will also unnecessarily stall a core and prevent any other tasks from running on that core even on CPUs with lots of cores, just that it's effect on the FPS will be lower.

Screenshots

Image 1 Htop output without the PR:
Screenshot from 2024-09-19 16-39-39

Image 2 Htop output with the PR (Default dispatcher is not even in the list when I sort by CPU usage):
Screenshot from 2024-09-19 17-10-28

Image 3 Htop output if i find DefaultDispatcher thread in the list with the PR (Uses 0-1% cpu):
Screenshot from 2024-09-19 17-09-58

Image 4 Spark report without the PR:
Screenshot_from_2024-09-15_07-45-36
Explanation: The isNotEmpty in kotlin gets compiled to a negated !isEmpty() call. isEmpty() in ConcurrentLinkedQueue calls first() and returns true if it's not null (source: https://github.com/openjdk/jdk8u/blob/master/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedQueue.java#L428) inside the while loop. There is no delay in the entire while loop since all methods called (isNotEmpty(), poll(), sendPacketAsync()) are non-blocking instant-finishing methods (poll() just returns null if no element is available - the method that blocks till element is available is take(), but it's only available in BlockingLinkedQueue, not ConcurrentLinkedQueue. An alternative fix was to make it BlockingLinkedQueue and call take(), but using blocking methods like take() inside coroutines would prevent another coroutine from using the shared thread so this fix is more feasible.).

While loop without a delay starves CPU time in background inside the coroutine's DefaultDispatcher thread, competing with Client thread and causing horrible lag inside dungeons on machines with low amount of CPU cores (<=4)

Add a delay(50) to fix it.

Signed-off-by: Mustafa ÖNCEL <[email protected]>
@TheDGOfficial TheDGOfficial marked this pull request as draft September 19, 2024 15:41
@TheDGOfficial
Copy link
Author

As per the recommendation from nea at https://discord.com/channels/807302538558308352/1277582383280951347/1286349093215539263 I will try to make this based on channels instead of a 50ms delay so I have marked it as draft.

However since this fixes most of the issue already and drops down CPU usage from 90-100% to 0-1% it can be merged in the meantime if needed and the channel instead of queue can be another PR.

Please let me know if you want me to keep it as draft and update it to use channels in this PR or proceed merging this one and open new one for channels.

@TheDGOfficial
Copy link
Author

Closed in place of #502

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.

1 participant