-
-
Notifications
You must be signed in to change notification settings - Fork 36
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
feat(async): add extra async versions of public APIs #6
Conversation
cda44d1
to
e0720b4
Compare
c909a08
to
a3e3d41
Compare
5b9e5f3
to
667bb50
Compare
7a0f904
to
74dce57
Compare
Update: this is ready to merge just as soon as |
@zkat heh, so if this is entirely bottlenecked by fs access then 2x perf drop sounds about what we've measured in the past. The reason for this is that in order for a blocking call to be wrapped in a future, an extra edit: it's probably worth pointing out that imo using async APIs is probably still worth it because we'll probably be able to figure out how to remove the perf drop, and likely eventually also include faster fs access through |
a491840
to
0ffa24b
Compare
No streaming support for now, because that'll be more tricky -- I want to make sure I get this PR in first. Streams will definitely be a big deal to add, and will more than justify the async perf hit because they'll be able to handle much bigger files. |
@zkat wooooot! 🎉 🎉 |
@yoshuawuyts also of interest might be that there was a significant boost in performance of the async version. It went from 34us to 24us on the same benchmarks. IDK what caused that change in perf. |
@zkat yay that's great to hear, thanks for sharing! We recently added a workstealing executor algorithm to async-std, which may explain some of the perf improvements you're seeing! |
(this PR is still a WIP - a few APIs still need porting)
This adds full async support to all relevant public cacache APIs, which should allow for concurrently handling a bunch of files.
I'm a bit concerned about the performance, but this might just be the overhead from
task::block_on
. I'm not really sure if it would literally double the time spent, though. That seems like a lot. /cc @stjepang @yoshuawuyts: