Skip to content

Add enable_time() when worker_threads="auto" #22

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

Closed
wants to merge 1 commit into from

Conversation

lxb007981
Copy link
Contributor

Since tokio::time::sleep is used in accept(), we need to add enable_time() in the runtime builder.

But I didn't get the idea why we chose to sleep here? In the current implementation, every time when tcp_socket.accept() is called, it will likely waste some time, return an Err and leave a trace message. If the listeners are many, it might harm the responsiveness. Maybe the better way is to spawn a separate task per listener to call listener.accept(), and see who comes first? Like using futures:future:select_all. Or even simpler, to only support the case with one single listener, like speedtest-go did. Correct me if I'm wrong. Thank you!

@SudoDios
Copy link
Collaborator

@lxb007981
You are very accurate 👏
It was because the iteration remains active. But it can probably be implemented better.
I am a bit busy these few days; Some places need cleaning

@SudoDios
Copy link
Collaborator

like speedtest-go did

The librespeed-go version does not support multiple listeners.
But here it is possible

@SudoDios SudoDios closed this May 13, 2025
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.

2 participants