Extend the polling feature to cover TryStream::try_read_inner()#76
Extend the polling feature to cover TryStream::try_read_inner()#76daniel-thompson wants to merge 3 commits intozhiburt:mainfrom
Conversation
The ready() method allows us to optionally perform a sleeping poll to determine if characters are available. This can be used, on platforms that support polling, to minimise CPU time when waiting for characters.
|
I like idea to add a timeout to Maybe adding a new method would be better? What do you think? PS: And it would be great to turn CI green :)
I can imagine if you are doing In theory you could use combination of But I see the problem that sometimes reading with timeout would be just easier. |
|
Reading things back I'll probably just drop the changes to
Could you kick of a CI run on the main branch? There are some failures caused by my changes but some look like they were already there (the Code Coverage / Coveralls check for example). |
ready() has a default "always-ready" implementation but we Unix systems we can use a Poller to make character polling more efficient. Let's do that!
Currently TryStream::try_read_inner() are part of a busy-loop that waits for characters. We can make that wait use significantly less CPU by sleeping until characters are available instead. This should have no impact on performance providing characters or EOF are received before reaching the timeout. The 100ms timeout could impact the responsiveness of Sessions with a short timeouts. Happily these sessions should be uncommon and their needs could be met by disabling the polling feature.
daniel-thompson
changed the title
|
Sorry for the close/reopen... it happened automatically when I removed my changes from my main branch to check the CI results without any of my changes. I have fixed the clippy warning that came from my changes but I'm not sure how best to fix the existing CI problems on MacOS and with the code coverage tool. |
|
I've reviewed the failures in the CI report for this branch and am confident none are introduced by the changes in this PR. The two macos-latest failures are observed in runs that do not contain my changes (https://github.com/daniel-thompson/expectrl/actions/runs/23892372808 ). The same is true of the code coverage checks (https://github.com/daniel-thompson/expectrl/actions/runs/23892372814). The main check is a credit check failing and I don't think any code from the PR has been downloaded at the point of failure. |
2 participants
The busy-loop when waiting for characters to be available is consuming far too much CPU (and power) for my use case.
These three patches introduce a new method to the
NonBlockingtrait making it possible to wait until data is ready. An "always ready" implementation is provided by default and should prevent this from being a breaking API change (although to use the new feature then code must be changed).On Unix systems we fully implement
NonBlocking::ready()and then use if fromsync_session.rsto avoid busy-waiting.