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

Explicit poll/flush operation #452

Closed
ndmitchell opened this issue Nov 12, 2022 · 8 comments · Fixed by #524
Closed

Explicit poll/flush operation #452

ndmitchell opened this issue Nov 12, 2022 · 8 comments · Fixed by #524
Assignees
Milestone

Comments

@ndmitchell
Copy link

This library seems to be a great fit for things like cargo watch which are continuously watching and waiting. However, it was a bit harder to integrate with a build system like Buck/Bazel, which typically want to know all changes at the start of a build (when the user types build), then build ignoring file system changes thereafter (or sometimes pick them up at the end to warn the user). That means that instead of having events that arrive at some point (potentially racing with other operations such as the user typing build), and instead of polling at a defined interval (for the polling watcher), it would be useful to have a poll or flush method on Watcher that ensured all in-flight notifications were delivered (e.g. flush the underlying file descriptors or similar) and in the case of the poll watcher, polled immediately at that point.

@0xpr03
Copy link
Member

0xpr03 commented Nov 12, 2022

I think I'm missing some context. Apart from PollWatcher, Notify doesn't decide when it emits events, it'll deliver events the moment they're coming in from the OS (minor some details, so at max 10ms delay). Because of that I'm not exactly sure what you're aiming at. For the PollWatcher we could add some special method to force-poll, but in general I'd say: Ignore events for the time you expect them to race (or collect them for later use), and maybe wait 10ms before starting that freeze-period if you want to make sure you've gotten all events prior to that point.

@ndmitchell
Copy link
Author

For the poll watcher it would definitely be useful to have a force polling now, rather than doing it every 30 seconds, since I only care about it at the poll point.

For the other watchers, if there was a flush/poll method that just waited 10ms (since that's the time you know you buffer for) that would be great. My worry is that one day the internals might change to being 100ms, and it would be great if things updated automatically. For notification methods that don't have any delay built in, doing nothing would be even better (although always waiting 10ms is no big deal either way).

@Chaoses-Ib
Copy link

In addition to working as a fence, explicit polling is also useful for implementing advanced scheduling. One can set the poll interval to infinity and manually poll at a specific time, such as at 0:00 every day.

@Chaoses-Ib
Copy link

#258 seems to be a more general case of this issue.

@0xpr03 0xpr03 self-assigned this Jun 20, 2023
@0xpr03 0xpr03 added this to the 6.1.0 milestone Jun 20, 2023
@0xpr03
Copy link
Member

0xpr03 commented Jun 20, 2023

There are two approaches to this:

Allow doing something like PollWatcher::poll(&self), which gives no response and just invokes the next cycle (value via callback).

Or PollWatcher::poll(&self) -> Result<Poll Values>, which is probably more complex to achieve and integrate into the existing code.

@Chaoses-Ib
Copy link

Chaoses-Ib commented Jul 12, 2023

I would vote for the first approach.

For the first approach, the implementation would simply be adding a channel (Mutex<mpsc::Sender>/Receiver or mpsc::SyncSender/Receiver due to the Send + Sync requirement). If we use the first approach, users can mimic the second interface by simply introducing a channel in the callback by themselves.

For the second approach, to prevent sync issues and reset the next run time, the implementation is likely to first notify the monitor thread to run via a channel, and then get the results back via another channel. If we use the second approach, users can still mimic the first interface by introducing one more channel. However, in this way the data is sent from the monitor thread to the caller and then back to the monitor thread, which is an unnecessary cost.

@0xpr03
Copy link
Member

0xpr03 commented Aug 12, 2023

Have a look at #524. This should give you manual polling where you can disable any automatic polling.

@0xpr03
Copy link
Member

0xpr03 commented Aug 13, 2023

Example here

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

Successfully merging a pull request may close this issue.

3 participants