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

Convert to Async #22

Merged
merged 18 commits into from
May 25, 2020
Merged

Conversation

jeremyandrews
Copy link
Member

@jeremyandrews jeremyandrews commented May 23, 2020

Re-implement async support without using macro_rules_attribute

With help from this thread:
https://www.reddit.com/r/rust/comments/goh2be/how_to_store_an_async_future_to_a_function_with_a/

alsuren and others added 10 commits May 23, 2020 08:15
It's a bit of a shitshow because of lifetimes, and the
fact that async functions can't be used as function
pointers (because the return value is not sized
predictably in a dynamic context).

This thread was really hepful to me:
https://users.rust-lang.org/t/how-to-store-async-function-pointer/38343/4

All that's left to do is:
* Fix the doctests
* Actually try out the examples and see if they are
  still working/performant.
* Go hunting for places where explicit threads are used
  which could be turned into tasks.
@jeremyandrews jeremyandrews mentioned this pull request May 23, 2020
@jeremyandrews jeremyandrews changed the title [WIP] Async without macro_rules_attribute macro [WIP] Async 2 May 24, 2020
@jeremyandrews
Copy link
Member Author

@alsuren I've added a simple macro to remove the code-duplication required to wrap each task. I couldn't figure out how to put task!() inside .register_task() as you'd proposed, any ideas are very welcome. I'd also appreciate any thoughts on how to improve the macro ergonomics.

While it's not perfect, as it gives a 2x boost in performance I think this is usable/mergable.

examples/drupal_loadtest.rs Outdated Show resolved Hide resolved
examples/drupal_loadtest.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@LionsAd LionsAd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good to merge - only nits remain

src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/goose.rs Outdated Show resolved Hide resolved
@jeremyandrews jeremyandrews changed the title [WIP] Async 2 Convert to Async May 25, 2020
@jeremyandrews jeremyandrews merged commit 54e1c27 into tag1consulting:master May 25, 2020
Copy link
Collaborator

@LionsAd LionsAd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarified comment

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

Successfully merging this pull request may close these issues.

3 participants