-
Notifications
You must be signed in to change notification settings - Fork 215
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
Add debouncer #286
Add debouncer #286
Conversation
9746530
to
b64cdf1
Compare
Open questions:
|
3361c3d
to
44c21ba
Compare
f621b5b
to
1c118ba
Compare
Another I just realized: We need an idea how we handle configuration changes for debouncer and immediate watchers. Currently we don't care at all. Historically every debouncer init was implemented at the specific notify modules (inotify, fsevent..). What I realized: we don't actually implement any of the current config options. Or am I wrong ? Searching for PreciseEvents doesn't emit one hit across any of the specific implementations. Also from all the config options exactly one is relevant to the specific implementations of the immediate watchers: PreciseEvents, everything else is related to the debouncer. Thus the whole ".config()" operation is nearly irrelevant for immediate watchers. cc @JohnTitor |
Notes from implementing the watchexec 2.0 debouncer:
For reference, algorithm I implemented (simplified): let throttle: Duration = unimplemented!("input: debouncer interval");
let mut last = Instant::now();
let mut set = Vec::new();
loop {
let maxtime = if set.is_empty() {
Duration::from_secs(u64::MAX)
} else {
throttle.saturating_sub(last.elapsed())
};
if maxtime.is_zero() {
if set.is_empty() {
last = Instant::now();
continue;
} else {
// we're out of the debouncing
}
} else {
match timeout(maxtime, events.recv()).await {
Err(_timeout) => continue,
Ok(None) => break, // exit
Ok(Some(event)) => {
// filtering applied here, if fail reset loop
if set.is_empty() {
last = Instant::now();
}
set.push(event);
if last.elapsed() < throttle {
continue;
}
}
}
}
// we're out of the debouncing, pass event set to consumer
last = Instant::now();
let pass_this = set.drain(..).collect();
} |
I am curious when will this be merged ? |
Once I've got time to fully undertake this as there is a lot I want to get right or at least decide actively to not include it, as you can read above. Which due to private things going on could take until march next year. I consider v4 to be pretty stable, so feel free to use that one. |
e515d57
to
8f8280c
Compare
This is the tiniest debouncer I could imagine being useful. Only Any events are delivered and AnyContinuous for ongoing events past the timeout. Things I don't like:
|
The real problem with all of these three is how we pass the configuration stuff back and forth. User -> backend (config), User -> debouncer (config), backend -> debouncer (events, errors), debouncer -> User (events, errors) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too familiar with it but the implementation looks great to me with a nit!
src/debouncer.rs
Outdated
) -> Result<(Receiver<DebounceChannelType>, RecommendedWatcher), Error> { | ||
let data = DebounceData::default(); | ||
|
||
let tick_div = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'd like to manage this value as const and declare it on the module scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understandable, had a moment of wrangling with myself whether I'd want that as top module declaration
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as abuse.
This comment was marked as abuse.
8a1d6e1
to
27634a4
Compare
This got a little bit bigger now. Fixing the audit issues and decoupling the debouncer from notify core. |
6736894
to
3f23588
Compare
As good as finished. |
4d7023b
to
00c7cba
Compare
Add debouncer back to v5.
Related #249