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

Use native POSIX polling syscalls to read input #624

Merged
merged 2 commits into from Jan 3, 2024

Conversation

lvxnull
Copy link
Contributor

@lvxnull lvxnull commented Sep 21, 2023

This will decrease the amount of thread synchronization calls during input processing and memory usage by removing the dedicated input thread and moving input reading code into the main thread, in polling read mode(non-canon, VMIN and VTIME set to 0), as well as replace the manual, sleep based polling implementation with pselect. Because polling is now done by pselect, SIGUSR1 will be used as the interrupt flag.
Additionally, input will be read in chunks instead of character by character.

@aristocratos
Copy link
Owner

@lvxnull
This is great, have you tested it on all platforms?

But I do believe the interrupt flag will still be needed as a way to signal that the next call to Input::poll should immediately return false. Because the the new Input::interrupt function will only stop a currently ongoing call to Input::poll right?

@lvxnull
Copy link
Contributor Author

lvxnull commented Sep 22, 2023

@aristocratos
I tested on linux only.

the new Input::interrupt function will only stop a currently ongoing call to Input::poll right?

No, because SIGUSR1 is only unblocked during pselect, and when a blocked signal is raised it stays in a pending state until it's unblocked and then executes immediately.

@aristocratos
Copy link
Owner

No, because SIGUSR1 is only unblocked during pselect, and when a blocked signal is raised it stays in a pending state until it's unblocked and then executes immediately.

Neat 👍🏼

Nice improvement over the previous mess!

Will do some stability testing on all platforms when I've got some time.

src/btop_input.cpp Outdated Show resolved Hide resolved
src/btop_input.cpp Outdated Show resolved Hide resolved
src/btop_input.cpp Outdated Show resolved Hide resolved
src/btop_input.cpp Outdated Show resolved Hide resolved
src/btop_input.cpp Outdated Show resolved Hide resolved
@imwints
Copy link
Contributor

imwints commented Sep 25, 2023

Also is there a reason to use select() over poll()?

@lvxnull
Copy link
Contributor Author

lvxnull commented Sep 25, 2023

Yes. ppoll() doesn't seem to be supported on mac os x. select()s file descriptor limitations don't apply here since only stdin is polled.

@imwints
Copy link
Contributor

imwints commented Sep 25, 2023

Can you update the declaration of poll(timeout)?

@lvxnull
Copy link
Contributor Author

lvxnull commented Sep 25, 2023

done

No more awkward manual polling
@aristocratos aristocratos merged commit 27c6f11 into aristocratos:main Jan 3, 2024
51 checks passed
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.

None yet

3 participants