Closed Bug 1606281 Opened 4 years ago Closed 4 years ago

Update dependency for hyper to 0.13 for async/await support

Categories

(Testing :: geckodriver, task, P3)

Version 3
task

Tracking

(firefox77 fixed)

RESOLVED FIXED
mozilla77
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.

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"

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/

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/

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.

geckodriver can handle multiple concurrent HTTP requests, but only a single WebDriver session. Those are different things.

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.

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.

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.

Pretty straightforward update. This duplicates the tokio stack because
audioipc hasn't been updated yet. (https://github.com/djg/audioipc-2/issues/97)

Assignee: nobody → eijebong
Status: NEW → ASSIGNED
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
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Blocks: 1584911

== 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

(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.

Flags: needinfo?(marian.raiciof)

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

Flags: needinfo?(marian.raiciof)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: