-
Notifications
You must be signed in to change notification settings - Fork 597
(WIP) Use AsioFuture to wait asynchronously on blocking tasks #10626
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
base: master
Are you sure you want to change the base?
Conversation
6ec7bae to
86ff3fc
Compare
86ff3fc to
5879e6f
Compare
| else if (request.method() == http::verb::delete_) | ||
| HandleDelete(request, response); | ||
| else | ||
| return false; |
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.
What about wrapping at the Handle*() level? I mean, what a waste to deploy a callback which will almost immediately return false..
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.
Absolutely. That's why I put an extra (WIP) in front of the second commit. It was just thrown together quickly to test if it even works. I'm sure it can be made a lot nicer by applying the callbacks more selectively.
Al2Klimov
left a comment
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 originally came here to write:
Maybe you can replace #10273
But I've seen your updated OP.
My strong opinion
I'm clearly against new Boost packages, unless we AbSoLuTeLy NeEd them!
I fully agree with you here. I won't argue for shipping boost packages on targets where we don't already need them anyway. The PR description is just to track which versions would be needed. |
This could be an alternative approach to get rid of the CpuBoundWork.
WIP for now, but discussions are welcome.
Blockers
std::exception_ptreven in the current version. I will report this to boost.asio, but on its own this wouldn't be too difficult to work around.