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

Replace hyper with tiny_http to serve http files for serve functionality #6042

Merged
merged 17 commits into from
Apr 22, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Apr 19, 2024

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

  • 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!
  • Test Jupyter notebook

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

@Wumpf Wumpf added dependencies concerning crates, pip packages etc 🚜 refactor Change the code, not the functionality include in changelog labels Apr 19, 2024
@Wumpf Wumpf added the do-not-merge Do not merge this PR label Apr 19, 2024
@Wumpf Wumpf marked this pull request as ready for review April 19, 2024 09:19
crates/re_web_viewer_server/src/lib.rs Outdated Show resolved Hide resolved
crates/re_web_viewer_server/src/lib.rs Outdated Show resolved Hide resolved
crates/re_web_viewer_server/src/lib.rs Outdated Show resolved Hide resolved
crates/re_web_viewer_server/src/lib.rs Show resolved Hide resolved
crates/re_web_viewer_server/src/lib.rs Outdated Show resolved Hide resolved
crates/re_web_viewer_server/src/lib.rs Outdated Show resolved Hide resolved
crates/re_web_viewer_server/src/lib.rs Outdated Show resolved Hide resolved
crates/rerun/src/run.rs Outdated Show resolved Hide resolved
web_viewer/index.html Show resolved Hide resolved
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.

Nice!

@Wumpf Wumpf removed the do-not-merge Do not merge this PR label Apr 22, 2024
@Wumpf Wumpf merged commit f70d675 into main Apr 22, 2024
33 checks passed
@Wumpf Wumpf deleted the andreas/replace-hyper-with-tiny-http branch April 22, 2024 13:38
Wumpf added a commit that referenced this pull request Apr 22, 2024
### What

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

* Fixes #5907
* Direct follow-up / merges into #6042

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:
* 0.15.1 already regressed quite a bit compared back to January:
#4824
* we currently have a few temporary crates while migrating to re_query2
which weren't present in 0.15

### 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/6043?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/6043?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)!

- [PR Build Summary](https://build.rerun.io/pr/6043)
- [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`.
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 🚜 refactor Change the code, not the functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants