-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 an event system #8021
Merged
Merged
Add an event system #8021
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,17 @@ | ||
# we use tokio_unstable to enable runtime::Handle::id so we can separate | ||
# globals from multiple parallel tests. If that function ever does get removed | ||
# its possible to replace (with some additional overhead and effort) | ||
# Annoyingly build.rustflags doesn't work here because it gets overwritten | ||
# if people have their own global target.<..> config (for example to enable mold) | ||
# specifying flags this way is more robust as they get merged | ||
# This still gets overwritten by RUST_FLAGS though, luckily it shouldn't be necessary | ||
# to set those most of the time. If downstream does overwrite this its not a huge | ||
# deal since it will only break tests anyway | ||
[target."cfg(all())"] | ||
rustflags = ["--cfg", "tokio_unstable", "-C", "target-feature=-crt-static"] | ||
|
||
|
||
[alias] | ||
xtask = "run --package xtask --" | ||
integration-test = "test --features integration --profile integration --workspace --test integration" | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
use std::future::Future; | ||
|
||
pub use oneshot::channel as cancelation; | ||
use tokio::sync::oneshot; | ||
|
||
pub type CancelTx = oneshot::Sender<()>; | ||
pub type CancelRx = oneshot::Receiver<()>; | ||
|
||
pub async fn cancelable_future<T>(future: impl Future<Output = T>, cancel: CancelRx) -> Option<T> { | ||
tokio::select! { | ||
biased; | ||
_ = cancel => { | ||
None | ||
} | ||
res = future => { | ||
Some(res) | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
//! Utilities for declaring an async (usually debounced) hook | ||
|
||
use std::time::Duration; | ||
|
||
use futures_executor::block_on; | ||
use tokio::sync::mpsc::{self, error::TrySendError, Sender}; | ||
use tokio::time::Instant; | ||
|
||
/// Async hooks provide a convenient framework for implementing (debounced) | ||
/// async event handlers. Most synchronous event hooks will likely need to | ||
/// debounce their events, coordinate multiple different hooks and potentially | ||
/// track some state. `AsyncHooks` facilitate these use cases by running as | ||
/// a background tokio task that waits for events (usually an enum) to be | ||
/// sent through a channel. | ||
pub trait AsyncHook: Sync + Send + 'static + Sized { | ||
type Event: Sync + Send + 'static; | ||
/// Called immediately whenever an event is received, this function can | ||
/// consume the event immediately or debounce it. In case of debouncing, | ||
/// it can either define a new debounce timeout or continue the current one | ||
fn handle_event(&mut self, event: Self::Event, timeout: Option<Instant>) -> Option<Instant>; | ||
|
||
/// Called whenever the debounce timeline is reached | ||
fn finish_debounce(&mut self); | ||
|
||
fn spawn(self) -> mpsc::Sender<Self::Event> { | ||
pascalkuthe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// the capacity doesn't matter too much here, unless the cpu is totally overwhelmed | ||
// the cap will never be reached since we always immediately drain the channel | ||
// so it should only be reached in case of total CPU overload. | ||
// However, a bounded channel is much more efficient so it's nice to use here | ||
let (tx, rx) = mpsc::channel(128); | ||
tokio::spawn(run(self, rx)); | ||
tx | ||
} | ||
} | ||
|
||
async fn run<Hook: AsyncHook>(mut hook: Hook, mut rx: mpsc::Receiver<Hook::Event>) { | ||
let mut deadline = None; | ||
loop { | ||
let event = match deadline { | ||
Some(deadline_) => { | ||
let res = tokio::time::timeout_at(deadline_, rx.recv()).await; | ||
match res { | ||
Ok(event) => event, | ||
Err(_) => { | ||
hook.finish_debounce(); | ||
deadline = None; | ||
continue; | ||
} | ||
} | ||
} | ||
None => rx.recv().await, | ||
}; | ||
let Some(event) = event else { | ||
break; | ||
}; | ||
deadline = hook.handle_event(event, deadline); | ||
} | ||
} | ||
|
||
pub fn send_blocking<T>(tx: &Sender<T>, data: T) { | ||
// block_on has some overhead and in practice the channel should basically | ||
// never be full anyway so first try sending without blocking | ||
if let Err(TrySendError::Full(data)) = tx.try_send(data) { | ||
// set a timeout so that we just drop a message instead of freezing the editor in the worst case | ||
let _ = block_on(tx.send_timeout(data, Duration::from_millis(10))); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
//! rust dynamic dispatch is extremely limited so we have to build our | ||
//! own vtable implementation. Otherwise implementing the event system would not be possible. | ||
//! A nice bonus of this approach is that we can optimize the vtable a bit more. Normally | ||
//! a dyn Trait fat pointer contains two pointers: A pointer to the data itself and a | ||
//! pointer to a global (static) vtable entry which itself contains multiple other pointers | ||
//! (the various functions of the trait, drop, size and align). That makes dynamic | ||
//! dispatch pretty slow (double pointer indirections). However, we only have a single function | ||
//! in the hook trait and don't need a drop implementation (event system is global anyway | ||
//! and never dropped) so we can just store the entire vtable inline. | ||
|
||
use anyhow::Result; | ||
use std::ptr::{self, NonNull}; | ||
|
||
use crate::Event; | ||
|
||
/// Opaque handle type that represents an erased type parameter. | ||
/// | ||
/// If extern types were stable, this could be implemented as `extern { pub type Opaque; }` but | ||
/// until then we can use this. | ||
/// | ||
/// Care should be taken that we don't use a concrete instance of this. It should only be used | ||
/// through a reference, so we can maintain something else's lifetime. | ||
struct Opaque(()); | ||
pascalkuthe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
pub(crate) struct ErasedHook { | ||
data: NonNull<Opaque>, | ||
call: unsafe fn(NonNull<Opaque>, NonNull<Opaque>, NonNull<Opaque>), | ||
} | ||
|
||
impl ErasedHook { | ||
pub(crate) fn new_dynamic<H: Fn() -> Result<()> + 'static + Send + Sync>( | ||
hook: H, | ||
) -> ErasedHook { | ||
unsafe fn call<F: Fn() -> Result<()> + 'static + Send + Sync>( | ||
hook: NonNull<Opaque>, | ||
_event: NonNull<Opaque>, | ||
result: NonNull<Opaque>, | ||
) { | ||
let hook: NonNull<F> = hook.cast(); | ||
let result: NonNull<Result<()>> = result.cast(); | ||
let hook: &F = hook.as_ref(); | ||
let res = hook(); | ||
ptr::write(result.as_ptr(), res) | ||
} | ||
|
||
unsafe { | ||
ErasedHook { | ||
data: NonNull::new_unchecked(Box::into_raw(Box::new(hook)) as *mut Opaque), | ||
call: call::<H>, | ||
} | ||
} | ||
} | ||
|
||
pub(crate) fn new<E: Event, F: Fn(&mut E) -> Result<()>>(hook: F) -> ErasedHook { | ||
unsafe fn call<E: Event, F: Fn(&mut E) -> Result<()>>( | ||
hook: NonNull<Opaque>, | ||
event: NonNull<Opaque>, | ||
result: NonNull<Opaque>, | ||
) { | ||
let hook: NonNull<F> = hook.cast(); | ||
let mut event: NonNull<E> = event.cast(); | ||
let result: NonNull<Result<()>> = result.cast(); | ||
let hook: &F = hook.as_ref(); | ||
let res = hook(event.as_mut()); | ||
ptr::write(result.as_ptr(), res) | ||
} | ||
|
||
unsafe { | ||
ErasedHook { | ||
data: NonNull::new_unchecked(Box::into_raw(Box::new(hook)) as *mut Opaque), | ||
call: call::<E, F>, | ||
} | ||
} | ||
} | ||
|
||
pub(crate) unsafe fn call<E: Event>(&self, event: &mut E) -> Result<()> { | ||
let mut res = Ok(()); | ||
|
||
unsafe { | ||
(self.call)( | ||
self.data, | ||
NonNull::from(event).cast(), | ||
NonNull::from(&mut res).cast(), | ||
); | ||
} | ||
res | ||
} | ||
} | ||
|
||
unsafe impl Sync for ErasedHook {} | ||
unsafe impl Send for ErasedHook {} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why is
crt-static
also enabled here now?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.
And why are we applying this flag in all cases now, could we only apply it for test builds?
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.
crt static is being disabled (not enabled that would be
+crt-static
instead of-crt-static
). The reason its here is that if we set RUST_FLAGS env variable then rusflags from.cargo/config.toml
gets overwritten. On some distros (like musl) it's necessary to disable static linking for helx to work (helix can never work with a statically linked libc).While its true that
tokio_unstable
is only needed during (integration test) the flag doesn't matter much (these tokio functions always exists as they are used internally, the flag just makes them pub). I tried coming up with a way to only enable during integration testing but that doesn't seem to work since you can't do cfg based on feature in cargo/config.toml only targets. cfg(test) doesn't work since cfg test only applied to the crate being tested - in this case helix-term - and not dependencies - in this case helix-event -