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

Parallelize space views #1325

Closed
emilk opened this issue Feb 16, 2023 · 4 comments · Fixed by #4460
Closed

Parallelize space views #1325

emilk opened this issue Feb 16, 2023 · 4 comments · Fixed by #4460
Assignees
Labels
📉 performance Optimization, memory use, etc 🔺 re_renderer affects re_renderer itself 📺 re_viewer affects re_viewer itself 🎄 tracking issue issue that tracks a bunch of subissues 🕸️ web regarding running the viewer in a browser

Comments

@emilk
Copy link
Member

emilk commented Feb 16, 2023

The Rerun viewer needs to run in Wasm on the browser. This limits our threading ability. std::thread is, for instance, not available. At the same time we want to make sure our viewer achieves good frame rate when compiled natively.

Parallelism on native, with single-threaded fallback on web

rayon is the go-to Rust crate for fine-grained parallelism. It is commonly used by library crates (behind a feature flag).
Currently rayon does not run in wasm in the browser, but this might change soon:

In the mean-time we can also define our own helpers for parallelism, e.g.:

/// Run two things in parallel
#[cfg(not(target_arch = "wasm32"))]
fn parallel<A: Send, B: Send>(
    a: impl Send + FnOnce() -> A,
    b: impl Send + FnOnce() -> B,
) -> (A, B) {
    rayon::join(a, b)
}

// Serial-fallback for wasm32
#[cfg(target_arch = "wasm32")]
fn parallel2<A: Send, B: Send>(
    a: impl Send + FnOnce() -> A,
    b: impl Send + FnOnce() -> B,
) -> (A, B) {
    ((a)(), (b)()) 
}

See #1041 for an experiment along these lines

Parallelism on wasm32

With a bit of work, one can spawn wasm32 threads in the browser with shared memory. Unfortunately, this requires a nightly compiler (for -Z build-std=std,panic_abort).

I think it could be fine for use to rely on the nightly compiler for just wasm32.

I suggest we keep this use-case in mind, but focus on the native performance for now.

Se also:

@emilk emilk added 🎄 tracking issue issue that tracks a bunch of subissues 📉 performance Optimization, memory use, etc labels Feb 16, 2023
@RReverser
Copy link

Parallelism on wasm32

Just to be clear, since it's not among links - you can do that via wasm-bindgen-rayon instead of implementing things by hand.

It does still require nightly compiler though.

@emilk
Copy link
Member Author

emilk commented Feb 28, 2023

Thanks @RReverser - I forgot to mention that! We experimented with wasm-bindgen-rayon last autumn but decided we didn't want to commit to nightly back then.

@RReverser
Copy link

Yeah fair enough, I wish rebuilding standard library wasn't necessary. There is one trick you can use to build even on stable, although it's not particularly recommended: https://twitter.com/RReverser/status/1608840842654908419

@emilk emilk added the 🕸️ web regarding running the viewer in a browser label Apr 26, 2023
@emilk
Copy link
Member Author

emilk commented Nov 6, 2023

We have started using rayon in a few places in the viewer, relying on its single-threaded fallback on web.

In order to get a proper speedup we need to make our RenderContext and ViewerContext Sync.

We want to pass in &ViewerContext instead of &mut ViewerContext everywhere, and have it be Sync and hence parallelizable.

@emilk emilk added the 🔺 re_renderer affects re_renderer itself label Nov 6, 2023
@emilk emilk changed the title Threading support More multi-threading Nov 6, 2023
@teh-cmc teh-cmc added the 📺 re_viewer affects re_viewer itself label Nov 21, 2023
@teh-cmc teh-cmc changed the title More multi-threading Parallelize space views Nov 21, 2023
@Wumpf Wumpf self-assigned this Nov 29, 2023
jleibs added a commit that referenced this issue Nov 29, 2023
### What
- Part of the #4308 stack:
  - THIS PR: #4310
  - #4311
  - #4380
  - #4381

Rather than doing queries on-demand, we now do all the queries up-front
when creating the ViewerContext.

This keeps us from re-running duplicate queries, and also eliminates the
need to maintain a parallel implementation of resolve instead of just
looking up the query results.

Probably best reviewed commit-by-commit.

This already hits some snags related to ViewerContext lifecycle that
will hopefully be addressed in
#1325

### 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 [demo.rerun.io](https://demo.rerun.io/pr/4310) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4310)
- [Docs
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Wumpf added a commit that referenced this issue Nov 29, 2023
…ity (#4387)

### What

* A step towards #1325

Hover state was already double buffered but selection was applied
immediately. Not it behaves the same way. This is not only faster but
also more consistent and gives us "safer" behavior

(branch name is a misnomer - had bigger plans and then split it)

### 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 [app.rerun.io](https://app.rerun.io/pr/4387) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4387)
- [Docs
preview](https://rerun.io/preview/27d7c248aef3221c98e1a477433d1f53e72887f7/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/27d7c248aef3221c98e1a477433d1f53e72887f7/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
teh-cmc pushed a commit that referenced this issue Nov 30, 2023
### What
- Part of the #4308 stack:
  - THIS PR: #4310
  - #4311
  - #4380
  - #4381

Rather than doing queries on-demand, we now do all the queries up-front
when creating the ViewerContext.

This keeps us from re-running duplicate queries, and also eliminates the
need to maintain a parallel implementation of resolve instead of just
looking up the query results.

Probably best reviewed commit-by-commit.

This already hits some snags related to ViewerContext lifecycle that
will hopefully be addressed in
#1325

### 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 [demo.rerun.io](https://demo.rerun.io/pr/4310) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4310)
- [Docs
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
teh-cmc pushed a commit that referenced this issue Nov 30, 2023
…ity (#4387)

### What

* A step towards #1325

Hover state was already double buffered but selection was applied
immediately. Not it behaves the same way. This is not only faster but
also more consistent and gives us "safer" behavior

(branch name is a misnomer - had bigger plans and then split it)

### 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 [app.rerun.io](https://app.rerun.io/pr/4387) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4387)
- [Docs
preview](https://rerun.io/preview/27d7c248aef3221c98e1a477433d1f53e72887f7/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/27d7c248aef3221c98e1a477433d1f53e72887f7/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Wumpf added a commit that referenced this issue Nov 30, 2023
…on-mutable everywhere (#4389)

### What

* Another step towards #1325
* Follow-up to #4387

### 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 [app.rerun.io](https://app.rerun.io/pr/4389) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4389)
- [Docs
preview](https://rerun.io/preview/405ddca2f8389bb62e1d1e65ef59a3d211361968/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/405ddca2f8389bb62e1d1e65ef59a3d211361968/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Wumpf added a commit that referenced this issue Nov 30, 2023
### What

The big highlight is `needless_pass_by_ref_mut` which helps our
parallization effort, which is why I declare this

* part of #1325

Commit by commit - each commit is one added lint & its fixes (or a
failure to do so!)

Selected from
https://rust-lang.github.io/rust-clippy/master/index.html#?groups=cargo,complexity,correctness,deprecated,nursery,pedantic,perf,restriction,style,suspicious&versions=lte:74,gte:72&levels=allow,none

### 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 [app.rerun.io](https://app.rerun.io/pr/4404) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4404)
- [Docs
preview](https://rerun.io/preview/92edc9f673bca7c2eb77d0688d559f2eb09e4b8f/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/92edc9f673bca7c2eb77d0688d559f2eb09e4b8f/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Wumpf added a commit that referenced this issue Dec 5, 2023
…peline/Shader/Layouts/etc.) (#4421)

### What

* Prerequisite for #1325
* Followed by #4422

The dynamic resource pool (textures, buffers, bindgroups) was already
interior mutable. The static resource pool (mostly append only pool)
however, still required mutable access. This PR addresses this.

The actual tricky part is the implications this has on the draw step:
`wgpu::RenderPass<'a>` expects all passed in references to live for
`'a`. This isn't much of a problem we have with resources from the
dynamic resource pool since there the handles are actually Arcs to the
underlying wgpu resources. However we can't do this with RenderPipelines
(the only relevant static resource for this case) though since we want
to be able to swap out what the handle points to for shader reloading.
This means we need to hold the lock read lock on render pipelines for
the entirety of the pass recording.
This is quite nice and easy for our ViewBuilder draws (== individual
space views), but hard for the "compositing step" where we draw into
egui's render pass. This is solved by taking out all render pipelines of
the lock and putting them back in at the start of the frame.
Once wgpu lifts this restrictions we can simplify things a bit!

(to be continued with follow-up PRs!)


### 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):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4421/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4421/index.html?manifest_url=https://app.rerun.io/version)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4421)
- [Docs
preview](https://rerun.io/preview/8e4b0d88d9d01df8121a84850b251c1840494695/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8e4b0d88d9d01df8121a84850b251c1840494695/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)


---
Part of series towards more multithreading in the viewer!
* #4387
* #4404
* #4389
* You are here ➡️ #4421
* #4422
* #4430
Wumpf added a commit that referenced this issue Dec 5, 2023
…xt (#4422)

### What

* Prerequisite for #1325
* Follow-up of #4421
* Followed by #4430

Make draw data (this is in essence resource submission to GPU! e.g. make
pointcloud ready to render) an operation that doesn't require a mutable
renderer context.

Two pieces to this:
* `FileResolver` resolve needs to be non-mut, this turned out to be
trivial
* `Renderer::get_or_create` had to be streamlined. Renderer creation is
now done directly on the context and holding a renderer implies holding
a read lock. Since Renderers are added **very** rarely, this isn't much
of a limitation!

This causes a large trickle of removing `&mut` and gets us _almost_ to
having a non-mutable render context reference on the Viewer context!

### 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):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4422/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4422/index.html?manifest_url=https://app.rerun.io/version
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4422)
- [Docs
preview](https://rerun.io/preview/5a1ad48f35f4cd28bb4dc4cd56f56ac289a50ab3/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/5a1ad48f35f4cd28bb4dc4cd56f56ac289a50ab3/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

---
Part of series towards more multithreading in the viewer!
* #4387
* #4404
* #4389
* #4421
* You are here ➡️ #4422
* #4430
Wumpf added a commit that referenced this issue Dec 5, 2023
### What

* Prerequisite for #1325
* Follow-up of #4422

Makes `ViewBuilder`'s draw method no longer require a mutable render
context, thus no longer requiring a mutable frame state on the render
context (which previously held the mutable viewbuilder!), thus finally
lifting the need to store mutable references to the render context from
the viewer context!

Having a non-mut draw method on ViewBuilder is _mostly_ straight forward
at this point. The main hurdle are render phases that so far consumed
themselves on drawing because of readback textures (which are still
self-consuming for good reasons).

### 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):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4430/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4430/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
- Useful for quick testing when changes do not affect examples in any
way
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4430)
- [Docs
preview](https://rerun.io/preview/965d71c3723e74947e2d063372d51d9ae89cc9dd/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/965d71c3723e74947e2d063372d51d9ae89cc9dd/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)


---
Part of series towards more multithreading in the viewer!
* #4387
* #4404
* #4389
* #4421
* #4422 
* You are here ➡️ #4430
Wumpf added a commit that referenced this issue Dec 5, 2023
### What

* Prerequisite for #1325
* Follow-up of #4430 

What it says on the tin!
No additional review required, just waiting for rust ci to be sure :)

### 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):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4430/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4430/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
- Useful for quick testing when changes do not affect examples in any
way
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4430)
- [Docs
preview](https://rerun.io/preview/cc13e9becf363d9f611cfc59c61b41d1135ef411/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/cc13e9becf363d9f611cfc59c61b41d1135ef411/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)


---
Part of series towards more multithreading in the viewer!
* #4387
* #4404
* #4389
* #4421
* #4422 
* #4430
* You are here ➡️  #4438
Wumpf added a commit that referenced this issue Dec 11, 2023
### What

* Fixes #1325 

Two levels of parallism actually:
* All context & part systems for a single space view each run in
parallel
* All space views now in parallel first setup their query and then kick
of their (parallized!) system execution

This gives us major speedups whenever:
* There is several heavy systems in a scene (e.g. a lot of points and a
lot of arrows)
* There is several space views

As such this is a very fundamental & widely applicable parallelization!


![image](https://github.com/rerun-io/rerun/assets/1220815/14bc6ca8-abb0-480b-91dc-2da51253dd92)
(a rather silly "points with arrows" scene duplicated a bunch of times.
Note that we still don't have any caching, so all these space views
might as well be animated individually!)

<img width="1421" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/57e13e15-9d8f-4a30-bf74-d8174676986d">
(busy rayon worker threads!)


Related things that are not parallel yet:
* ui methods per space view (run one by one)
* draw call recording (done as part of the ui method, but we could
likely do this in parallel, only joining on these tasks when egui wants
to queue up the command buffers)
* processing _within_ a system depends on the system at hand. Today,
only point cloud has some parallization


### 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):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4460/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4460/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
- Useful for quick testing when changes do not affect examples in any
way
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4460)
- [Docs
preview](https://rerun.io/preview/cb7e9a9902bb9923ade41271ccc43ed01a2a4d33/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/cb7e9a9902bb9923ade41271ccc43ed01a2a4d33/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

---
End of a series on refactors leading to this parallization:
* #4387
* #4404
* #4389
* #4421
* #4422 
* #4430
* #4438
* #4460

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📉 performance Optimization, memory use, etc 🔺 re_renderer affects re_renderer itself 📺 re_viewer affects re_viewer itself 🎄 tracking issue issue that tracks a bunch of subissues 🕸️ web regarding running the viewer in a browser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants