Skip to content

Improve bundling of web-viewer package #6659

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

Merged
merged 16 commits into from
Jul 1, 2024
Merged

Improve bundling of web-viewer package #6659

merged 16 commits into from
Jul 1, 2024

Conversation

jprochazk
Copy link
Member

@jprochazk jprochazk commented Jun 26, 2024

What

It turns out that the following issue is actually two different memory leaks in a trench coat:

This PR addresses the 2nd one, by changing the way we output and bundle the rerun-io/web-viewer package:

  • We use the new URL("./local_file.js", import.meta.url) pattern to resolve the .wasm file's URL at bundle time, and fetch it at runtime. (Side note: we can use this to expose progress tracking for Slow loading without indicator gradio-rerun-viewer#5)
    • This understood by all major bundlers as "include local_file.js in the import graph as if it was loaded lazily, and return its final URL"
  • After building the Wasm binary and running wasm-bindgen, the output is further processed to ensure that the top-level globals (such as the leak-causing heap Memory leak when repeatedly instantiating a WASM module rustwasm/wasm-bindgen#3130) are not actually global, and instead can be instantiated once per Wasm instance.
  • The Wasm is loaded and compiled only once, and then instantiated once for every viewer.

To support usage in rerun_notebook and other environments which do not support the new URL pattern, the index.js, re_viewer.js, and re_viewer_bg.wasm files are concatenated into a single inlined.js file.

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.

@jprochazk jprochazk marked this pull request as ready for review June 28, 2024 11:25
@emilk emilk self-requested a review June 28, 2024 12:45
emilk
emilk approved these changes Jun 28, 2024
@jprochazk jprochazk added 🕸️ web regarding running the viewer in a browser notebook Jupyter notebooks etc exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jul 1, 2024
@jprochazk jprochazk merged commit 7b260c3 into main Jul 1, 2024
35 checks passed
@jprochazk jprochazk deleted the jan/postprocess-wbg branch July 1, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md notebook Jupyter notebooks etc 🕸️ web regarding running the viewer in a browser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants