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

Remove need for tokio runtime for supporting serve #6043

Merged
merged 4 commits into from
Apr 22, 2024
Merged

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Apr 19, 2024

What

Rust users now no longer need to ensure to have tokio runtime set up when using serve.

Last PR in a series of PRs for removal of the tokio runtime:

  • removes need for tokio in re_sdk_comms
    *similar to the refactor in re_ws_comms only in spirit - a lot simpler: no fancy broadcast, no other libraries involved. Just a bunch of threads hammering on blocking sockets).
  • remove remaining usages of async in this context
  • removes tokio need from all documentation

Dependency count 0.15.1 -> last friday d90ed2f7e-> this pr

  • cargo build -p rerun --no-default-features 225 -> 274 -> 275 dependencies
  • cargo build -p rerun -F default 361 -> 364 -> 364 dependencies
  • cargo build -p rerun -F web_viewer 409 -> 412 -> 374 dependencies

Notes:

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@Wumpf Wumpf added 🦀 Rust API Rust logging API do-not-merge Do not merge this PR dependencies concerning crates, pip packages etc include in changelog labels Apr 19, 2024
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

🍾

@@ -218,7 +218,6 @@ tiny_http = { version = "0.12", default-features = false }
tinystl = { version = "0.0.3", default-features = false }
tinyvec = { version = "1.6", features = ["alloc", "rustc_1_55"] }
tobj = "4.0"
tokio = { version = "1.24", default-features = false }
Copy link
Member

Choose a reason for hiding this comment

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

🥳

@Wumpf Wumpf force-pushed the andreas/remove-tokio branch from 18b7500 to 1672a7e Compare April 22, 2024 10:56
Base automatically changed from andreas/replace-hyper-with-tiny-http to main April 22, 2024 13:38
Wumpf added a commit that referenced this pull request Apr 22, 2024
…ality (#6042)

### What

The prime motivator for this is to fix the need for setting up a tokio
runtime every time someone calls serve.

Pro:
* no more need for tokio due to http server
* smaller dependency overall
* very simple lowlevel code for what should be also a very simple task
("send these 4 files whenever someone asks you")

Con:
* wasm is no longer sent compressed since tiny_http doesn't support this
* I had to do some odd hack in order to keep the wasm download progress
bar running (see comments on `rerun-wasm-size:` http header)
* this is not noticable on localhost, I only noticed this family of
issues by simulating lower transfer speeds in my browser

There's some loose ends for final tokio removal which are tied up in
#6043
(testing was mostly done on that pr and a little bit in isolation here
as well to track down the download progress bar issues)

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6042?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6042?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] Test Jupyter notebook

- [PR Build Summary](https://build.rerun.io/pr/6042)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
@Wumpf Wumpf force-pushed the andreas/remove-tokio branch from 1672a7e to 60cd72b Compare April 22, 2024 13:40
@Wumpf Wumpf removed the do-not-merge Do not merge this PR label Apr 22, 2024
@Wumpf Wumpf merged commit 2f73825 into main Apr 22, 2024
33 checks passed
@Wumpf Wumpf deleted the andreas/remove-tokio branch April 22, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies concerning crates, pip packages etc include in changelog 🦀 Rust API Rust logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider getting rid of tokio
2 participants