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

High CPU usage on HTTPClient with threads (on slow server response) #33479

Open
AlexHolly opened this issue Nov 9, 2019 · 5 comments
Open

High CPU usage on HTTPClient with threads (on slow server response) #33479

AlexHolly opened this issue Nov 9, 2019 · 5 comments

Comments

@AlexHolly
Copy link
Contributor

Godot version:
0ab0d11

OS/device including version:
Tested on Windows 10 and x11(Mint)

Issue description:
Maybe related to #32807
Using Theads to run HTTPClient Requests in the background is using 13-40% CPU (Reason is probably a while/requesting data from socket all the time instead of blocking until data is actually available)

I added a variable which reduces the CPU to ~ 0 - 5%
Currently it is always false.

return read(p_buffer, p_bytes, r_received, false);

Steps to reproduce:
Run example project
Check Taskmanager CPU usage or listen to your CPU fan :D

Minimal reproduction project:
http-bug.zip

Let me know if you think it is a bug I will open a PR with my solution.

This line should block until data comes in.

err = connection->get_partial_data(p_buffer + r_received, left, read);

Solution:
err = connection->get_partial_data(p_buffer + r_received, left, read, true);

@Faless
Copy link
Collaborator

Faless commented Nov 9, 2019

I think I understand the problem, and this is probably a defect introduced in #21976 .
We can't block in get_partial_data (which is supposed to be non-blocking).
get_data should be used instead, but due to the current API, we can't use that there either:

if (blocking) {
// We can't use StreamPeer.get_data, since when reaching EOF we will get an
// error without knowing how many bytes we received.
Error err = ERR_FILE_EOF;
int read = 0;

A proper fix might be possible without breaking the upper API nor reintroducing the EOF read bug, but this needs further investigation.

@Faless
Copy link
Collaborator

Faless commented Nov 21, 2019

@AlexHolly can you try out #33785 and and let us know if it fixes your issue?

@AlexHolly
Copy link
Contributor Author

It solves my case.

Do you have an idea why downloading a file still needs so much CPU?
#32807
Tested it with HTTPClient and HTTPRequest

@KoBeWi
Copy link
Member

KoBeWi commented Oct 19, 2020

Seems somewhat still valid in 3.2.3
The CPU usage is high (20%), but then goes down to standard ~2%.

@Faless
Copy link
Collaborator

Faless commented Oct 20, 2020

I need to salvage #33785 to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants