Minimal async-await foundations #709

ghost · 2021-03-12T19:03:02Z

This sets the foundations for async-await support in godot-rust, based on the original idea in #284. However, although the tests work, this is not a full implementation:

Async methods can only be registered manually using build_method. Macro syntax and implementation are out of the scope of this PR.
The runtime types aren't registered automatically yet. Users need to manually call register_runtime and terminate_runtime functions in their library lifecycle hooks. Improving this is out of the scope of this PR for now.
The crate is currently re-exported as gdnative::asn, instead of the much longer async_yield. The name is open to discussion -- I don't like it very much.
Only local spawners are supported, due to issues with thread safety. Users may off-load tasks that don't contain yield-likes to thread pool spawners using something like futures::future::Remote, however.
Panics in async methods don't currently behave very well. Their FunctionState-likes simply block forever and any outstanding bridge objects for futures can be leaked.

While the feature is not yet complete, the commit is already pretty big, and I feel that it's in a somewhat usable state. As a result, I'm putting this up as a draft PR to gather some feedback. If you have uses for async-await / "yield" from GDScript, please feel free to try it and tell me what you think!

Registering an async method currently looks like this (excerpt from the tests):

struct ResumeAddFn;

impl AsyncMethod<AsyncMethods> for ResumeAddFn {
    fn spawn_with(&self, spawner: Spawner<'_, AsyncMethods>) {
        spawner.spawn(|ctx, _this, mut args| {
            let a = args.read::<i32>().get().unwrap();
            let obj = args.read::<Ref<Object>>().get().unwrap();
            let name = args.read::<GodotString>().get().unwrap();

            async move {
                let b = ctx.until_resume().await;
                let b = i32::from_variant(&b).unwrap();

                let c = unsafe { obj.assume_safe().call(name, &[]) };
                let c = Ref::<Reference>::from_variant(&c).unwrap();
                let c = unsafe { c.assume_safe() };
                let c = ctx.signal(c, "completed").unwrap().await;
                assert_eq!(1, c.len());
                let c = i32::from_variant(&c[0]).unwrap();

                (a + b + c).to_variant()
            }
        });
    }
}

fn register_methods(builder: &ClassBuilder<AsyncMethods>) {
    builder
        .build_method("resume_add", Async::new(ResumeAddFn))
        .done();
}

Using it is almost like any other GDScript coroutine:

func _async_call(obj):
	var fn_state = obj.resume_add(1, self, "_get_async_number")
	# the "resumable" signal is unique to Rust, since unlike GDScript coroutines,
	# Rust futures aren't guaranteed to be polled up to a yield right after spawning.
	yield(fn_state, "resumable")
	fn_state = fn_state.resume(2)
	# no "resumable" signal when awaiting signals, though
	var result = yield(fn_state, "completed")
	assert(result == 42)

func _get_async_number():
	yield(get_tree().create_timer(0.1), "timeout")
	return 39

ghost · 2021-04-15T11:41:41Z

Merging this since there doesn't seem to be any objections.

bors r+

701: Add automatically_derived and allow lints in macros r=toasteater a=toasteater This should prevent clippy lints from showing up in user code in the future. See: - https://doc.rust-lang.org/reference/attributes/derive.html - https://doc.rust-lang.org/rustc/lints/groups.html - https://github.com/rust-lang/rust-clippy#clippy 709: Minimal async-await foundations r=toasteater a=toasteater This sets the foundations for async-await support in godot-rust, based on the original idea in #284. However, although the tests work, this is not a full implementation: - Async methods can only be registered manually using `build_method`. Macro syntax and implementation are out of the scope of this PR. - The runtime types aren't registered automatically yet. Users need to manually call `register_runtime` and `terminate_runtime` functions in their library lifecycle hooks. Improving this is out of the scope of this PR for now. - The crate is currently re-exported as `gdnative::asn`, instead of the much longer `async_yield`. The name is open to discussion -- I don't like it very much. - Only local spawners are supported, due to issues with thread safety. Users may off-load tasks that don't contain `yield`-likes to thread pool spawners using something like `futures::future::Remote`, however. - Panics in async methods don't currently behave very well. Their `FunctionState`-likes simply block forever and any outstanding bridge objects for futures can be leaked. - - - While the feature is not yet complete, the commit is already pretty big, and I feel that it's in a somewhat usable state. As a result, I'm putting this up as a draft PR to gather some feedback. If you have uses for async-await / "yield" from GDScript, please feel free to try it and tell me what you think! Registering an async method currently looks like this (excerpt from the tests): ```rust struct ResumeAddFn; impl AsyncMethod<AsyncMethods> for ResumeAddFn { fn spawn_with(&self, spawner: Spawner<'_, AsyncMethods>) { spawner.spawn(|ctx, _this, mut args| { let a = args.read::<i32>().get().unwrap(); let obj = args.read::<Ref<Object>>().get().unwrap(); let name = args.read::<GodotString>().get().unwrap(); async move { let b = ctx.until_resume().await; let b = i32::from_variant(&b).unwrap(); let c = unsafe { obj.assume_safe().call(name, &[]) }; let c = Ref::<Reference>::from_variant(&c).unwrap(); let c = unsafe { c.assume_safe() }; let c = ctx.signal(c, "completed").unwrap().await; assert_eq!(1, c.len()); let c = i32::from_variant(&c[0]).unwrap(); (a + b + c).to_variant() } }); } } fn register_methods(builder: &ClassBuilder<AsyncMethods>) { builder .build_method("resume_add", Async::new(ResumeAddFn)) .done(); } ``` Using it is almost like any other GDScript coroutine: ```gdscript func _async_call(obj): var fn_state = obj.resume_add(1, self, "_get_async_number") # the "resumable" signal is unique to Rust, since unlike GDScript coroutines, # Rust futures aren't guaranteed to be polled up to a yield right after spawning. yield(fn_state, "resumable") fn_state = fn_state.resume(2) # no "resumable" signal when awaiting signals, though var result = yield(fn_state, "completed") assert(result == 42) func _get_async_number(): yield(get_tree().create_timer(0.1), "timeout") return 39 ``` Co-authored-by: toasteater <[email protected]>

bors · 2021-04-15T12:00:31Z

Build failed (retrying...):

ci

709: Minimal async-await foundations r=toasteater a=toasteater This sets the foundations for async-await support in godot-rust, based on the original idea in #284. However, although the tests work, this is not a full implementation: - Async methods can only be registered manually using `build_method`. Macro syntax and implementation are out of the scope of this PR. - The runtime types aren't registered automatically yet. Users need to manually call `register_runtime` and `terminate_runtime` functions in their library lifecycle hooks. Improving this is out of the scope of this PR for now. - The crate is currently re-exported as `gdnative::asn`, instead of the much longer `async_yield`. The name is open to discussion -- I don't like it very much. - Only local spawners are supported, due to issues with thread safety. Users may off-load tasks that don't contain `yield`-likes to thread pool spawners using something like `futures::future::Remote`, however. - Panics in async methods don't currently behave very well. Their `FunctionState`-likes simply block forever and any outstanding bridge objects for futures can be leaked. - - - While the feature is not yet complete, the commit is already pretty big, and I feel that it's in a somewhat usable state. As a result, I'm putting this up as a draft PR to gather some feedback. If you have uses for async-await / "yield" from GDScript, please feel free to try it and tell me what you think! Registering an async method currently looks like this (excerpt from the tests): ```rust struct ResumeAddFn; impl AsyncMethod<AsyncMethods> for ResumeAddFn { fn spawn_with(&self, spawner: Spawner<'_, AsyncMethods>) { spawner.spawn(|ctx, _this, mut args| { let a = args.read::<i32>().get().unwrap(); let obj = args.read::<Ref<Object>>().get().unwrap(); let name = args.read::<GodotString>().get().unwrap(); async move { let b = ctx.until_resume().await; let b = i32::from_variant(&b).unwrap(); let c = unsafe { obj.assume_safe().call(name, &[]) }; let c = Ref::<Reference>::from_variant(&c).unwrap(); let c = unsafe { c.assume_safe() }; let c = ctx.signal(c, "completed").unwrap().await; assert_eq!(1, c.len()); let c = i32::from_variant(&c[0]).unwrap(); (a + b + c).to_variant() } }); } } fn register_methods(builder: &ClassBuilder<AsyncMethods>) { builder .build_method("resume_add", Async::new(ResumeAddFn)) .done(); } ``` Using it is almost like any other GDScript coroutine: ```gdscript func _async_call(obj): var fn_state = obj.resume_add(1, self, "_get_async_number") # the "resumable" signal is unique to Rust, since unlike GDScript coroutines, # Rust futures aren't guaranteed to be polled up to a yield right after spawning. yield(fn_state, "resumable") fn_state = fn_state.resume(2) # no "resumable" signal when awaiting signals, though var result = yield(fn_state, "completed") assert(result == 42) func _get_async_number(): yield(get_tree().create_timer(0.1), "timeout") return 39 ``` Co-authored-by: toasteater <[email protected]>

bors · 2021-04-15T12:18:25Z

Build failed:

ci

halzy

Just a few comments, will spend more time reviewing the PR.

gdnative-async/src/method.rs

gdnative-async/src/future.rs

gdnative-async/src/method.rs

gdnative-async/src/rt.rs

gdnative-async/Cargo.toml

This sets the foundations for async-await support in godot-rust, based on the original idea in #284. However, although the tests work, this is not a full implementation: - Async methods can only be registered manually using `build_method`. Macro syntax and implementation are out of the scope of this PR. - The runtime types aren't registered automatically yet. Users need to manually call `register_runtime` and `terminate_runtime` functions in their library lifecycle hooks. Improving this is out of the scope of this PR for now. - The crate is currently re-exported as `gdnative::asn`, instead of the much longer `async_yield`. The name is open to discussion -- I don't like it very much. - Only local spawners are supported, due to issues with thread safety. Users may off-load tasks that don't contain `yield`-likes to thread pool spawners using something like `futures::future::Remote`, however. - Panics in async methods don't currently behave very well. Their `FunctionState`-likes simply block forever and any outstanding bridge objects for futures can be leaked.

ghost · 2021-04-16T06:01:50Z

Updated.

bors try

bors · 2021-04-16T06:20:14Z

try

Build succeeded:

ci

halzy

PR looks great! Just some minor questions.

gdnative-async/src/rt/bridge.rs

halzy · 2021-04-16T14:03:21Z

gdnative-async/src/rt/bridge.rs

+            let id = pool.next_id;
+            pool.next_id += 1;


A bit of nitpicking here, this could be impl'd on pool.

gdnative-async/src/rt/func_state.rs

test/project/main.gd

test/src/test_async.rs

halzy · 2021-04-19T13:52:05Z

gdnative-async/src/rt.rs

+    if called {
+        Ok(())
+    } else {
+        Err(InitError::new())


When loaded in the Editor and one is iterating on a godot-rust project, the register_runtime() function is called more than once. This causes the InitError to be returned.

halzy · 2021-04-19T13:56:03Z

gdnative-async/src/executor.rs

+
+/// Sets the global executor for the current thread to a `&'static dyn LocalSpawn`.
+pub fn set_executor(sp: &'static dyn LocalSpawn) -> Result<(), SetExecutorError> {
+    LOCAL_SPAWN.with(|cell| cell.set(sp).map_err(|_| SetExecutorError::new()))


Same issue here, when running in the editor this will be called more than once when iterating.

ghost · 2021-06-11T11:14:26Z

Seems like the init issues are related to #746?

ghost · 2021-07-05T13:32:32Z

Blocked by #757: the init errors are caused by bugs / new behaviors in Godot 3.3.

It'd imo also be acceptable to require reloadable = false for this early version, if we want to merge this right now, since it isn't intended to be the final interface for most use cases.

Bromeon · 2021-10-25T16:22:56Z

Superseded by #804.

804: Minimal async-await foundations, updated r=Bromeon a=chitoyuu This is an updated and rebased version of #709, that is no longer blocked on #757. Notable changes outside the original PR include: - `TerminateInfo` and `InitializeInfo` are moved back into the public API. These types are used as arguments to custom init/terminate callbacks. Custom callbacks are necessary for calling the `gdnative_async` `register` and `terminate` functions. Currently, they are just moved into the top level of `gdnative_core`, which I think makes sense given their usage in top-level callbacks. Please let me know if a better place is available. - Renamed the top-level re-export from `asn` to `tasks`, since the original author apparently didn't like it. Supersedes #709 Co-authored-by: Chitose Yuuzaki <[email protected]> Co-authored-by: toasteater <[email protected]>

ghost added the feature Adds functionality to the library label Mar 12, 2021

ghost marked this pull request as ready for review March 22, 2021 11:38

halzy reviewed Apr 16, 2021

View reviewed changes

gdnative-async/src/method.rs Outdated Show resolved Hide resolved

gdnative-async/src/future.rs Outdated Show resolved Hide resolved

gdnative-async/src/method.rs Show resolved Hide resolved

gdnative-async/src/rt.rs Show resolved Hide resolved

gdnative-async/Cargo.toml Show resolved Hide resolved

bors bot added a commit that referenced this pull request Apr 16, 2021

Try #709:

b390301

halzy approved these changes Apr 16, 2021

View reviewed changes

halzy reviewed Apr 19, 2021

View reviewed changes

ghost added this to the 0.10 milestone Apr 27, 2021

ghost added the status: postponed label Jul 5, 2021

chitoyuu mentioned this pull request Oct 25, 2021

Minimal async-await foundations, updated #804

Merged

Bromeon closed this Oct 25, 2021

Bromeon added status: duplicate and removed status: postponed labels Oct 25, 2021

Minimal async-await foundations #709

Minimal async-await foundations #709

Uh oh!

Conversation

ghost commented Mar 12, 2021

Uh oh!

ghost commented Apr 15, 2021

Uh oh!

bors bot commented Apr 15, 2021

Uh oh!

bors bot commented Apr 15, 2021

Uh oh!

halzy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Apr 16, 2021

Uh oh!

bors bot commented Apr 16, 2021

try

Uh oh!

halzy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

halzy Apr 16, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

halzy Apr 19, 2021

Choose a reason for hiding this comment

Uh oh!

halzy Apr 19, 2021

Choose a reason for hiding this comment

Uh oh!

ghost commented Jun 11, 2021

Uh oh!

ghost commented Jul 5, 2021

Uh oh!

Bromeon commented Oct 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants