Update dependency for hyper to 0.13 for async/await support
Categories
(Testing :: geckodriver, task, P3)
Tracking
(firefox77 fixed)
Tracking | Status | |
---|---|---|
firefox77 | --- | fixed |
People
(Reporter: whimboo, Assigned: eijebong)
References
Details
Attachments
(2 files)
hyper 0.13 has been released early in December with async/await support in tokio 0.2.
We should plan our update, which might include a bit of extra work to also make geckodriver async/await based.
Reporter | ||
Comment 1•4 years ago
|
||
Also tokio is now a light-weight dependency and instead of pulling extra 43 crates as dependencies which adds extra time for compilation, it's now a single "tokio" create. Components are opt-in
now and need to be enabled via the feature
flag. Hyper only requests the sync
feature, so compilation time for that crate will drastically drop. dev-dependencies for tokio look like but those are only for our test jobs:
tokio = { version = "0.2.2", features = ["fs", "macros", "io-std", "rt-util", "sync", "time", "test-util"] }
tokio-test = "0.2"
Reporter | ||
Comment 2•4 years ago
|
||
Maybe this blog post could help a bit, which is based on porting a CLI app to the new hyper:
https://zupzup.org/async-awaitify-rust-cli/
Assignee | ||
Comment 3•4 years ago
|
||
Right now we're waiting for warp to get released. The master branch is already using hyper 0.13. I'll bug seanmonstar a bit about the release to get an estimation for a release date/
Comment 4•4 years ago
|
||
I agree we should update our deps, but since geckodriver intentionally only handles a single connection I'd be rather surprised if we want any async/await in the codebase.
Comment 5•4 years ago
|
||
geckodriver can handle multiple concurrent HTTP requests, but only a single WebDriver session. Those are different things.
Comment 6•4 years ago
|
||
Right, but apart from responding to HEAD
requests or returning HTTP-level errors we can't do anything with a second request; there's a global lock you have to acquire at https://searchfox.org/mozilla-central/source/testing/webdriver/src/server.rs#300 So I still don't see what advantage there is adding any complexity to handle multiple connections. Unless our libraries literally don't support a different mode of operation I'm still rather skeptical of moving to async/await when our expected number of connections is exactly one and our absolute worst case scenarios (like a bunch of clients using HEAD
requests to poll for readiness) are comfortably handled by threads.
Comment 7•4 years ago
|
||
As I understand it we already use the scheduling offered through older Tokio APIs and the latest release is just about modernising those concepts on top of the new async/
await` primitives that Rust now has. I don’t have any insight as to whether hyper supports a non-async, non-Tokio mode of operation.
Comment 8•4 years ago
|
||
Sure, if the alternatives are using a different library or using async/await, it will presumably be much less work to do the latter. But I'm hoping the changes for the upgrade are minimal because from the specific point of view of geckodriver, almost all of the futures stuff is just overhead given we don't allow non-trivial concurrency at the application level.
Assignee | ||
Comment 9•4 years ago
|
||
Pretty straightforward update. This duplicates the tokio stack because
audioipc hasn't been updated yet. (https://github.com/djg/audioipc-2/issues/97)
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D71462
Comment 12•4 years ago
|
||
Pushed by eijebong@bananium.fr: https://hg.mozilla.org/integration/autoland/rev/c65feea49211 Part 1: Update warp to 0.2. r=jgraham,webdriver-reviewers https://hg.mozilla.org/integration/autoland/rev/e68f50824460 Part 2: Revendor dependencies. r=jgraham
Comment 13•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c65feea49211
https://hg.mozilla.org/mozilla-central/rev/e68f50824460
Comment 14•4 years ago
|
||
== Change summary for alert #25765 (as of Wed, 29 Apr 2020 12:22:36 GMT) ==
Improvements:
8% raptor-ares6-firefox macosx1014-64-shippable opt 59.04 -> 54.28
4% raptor-ares6-firefox linux64-shippable opt 46.42 -> 44.73
3% raptor-ares6-firefox windows7-32-shippable opt 47.94 -> 46.39
2% raptor-ares6-firefox windows7-32-shippable opt 47.59 -> 46.63
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=25765
Reporter | ||
Comment 15•4 years ago
•
|
||
(In reply to Marian Raiciof [:marauder] from comment #14)
== Change summary for alert #25765 (as of Wed, 29 Apr 2020 12:22:36 GMT) ==
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=25765
Really? This is a change to geckodriver, which is only in use by Browsertime for Raptor. And the above test is a benchmark test which clearly doesn't run with Browsertime yet.
Comment 16•4 years ago
|
||
Thanks Henrik for the feedback!
I changed the alert to a datapoint on the left, that belongs to this bug :
https://bugzilla.mozilla.org/show_bug.cgi?id=1632534
Description
•