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

Prevent blueprint store from growing with every new space-view and container #8249

Open
emilk opened this issue Nov 29, 2024 · 0 comments
Open
Labels
🚀 performance Optimization, memory use, etc ⛃ re_datastore affects the datastore itself

Comments

@emilk
Copy link
Member

emilk commented Nov 29, 2024

Each space view and container produces a unique entity in the blueprint store.

If the users decides to remove a view or a container, we still keep its entity in the blueprint store, so that they can undo their action.
Over time, this can fill up the store with a lot of "legacy" entities, that are older than the undo buffer, but that cannot be collected by the GC. That's because the GC of the blueprint store is configured to always keep the last component of each entity. This is important, or we would forget about things in the blueprint that hasn't changed in a while.

I see two potential solution to this:

Mark-and-sweep (bespoke and complicated user-space solution)

We haver an explicit mark-and-sweep-pass where we mark everything that is reachable from any root container in the undo buffer.

This requires visiting all root containers in the undo buffer, and then recursively visiting all their children, at the right time step (the one of the root container), and then marking anything reached.

Clear-aware store GC (the nice and reusable solution)

When a user deletes a container or view, we log a recursive clear (that was already added in #7546 in anticipation of this issue). If we could make the store GC aware of clear, we could configure it to do something like the following:

  • If protect_latest=1, and the last thing logged is a recursive clear, that only keep that clear
  • In a separate step, remove all entities that has no components in the store, excepting clears

Thus the recursive clear would be a signal to the store GC that "if everything before the clear has been garbage-collected, then you can remove the whole entity". Of course this should configurable in GarbageCollectionOptions

@emilk emilk added enhancement New feature or request ⛃ re_datastore affects the datastore itself 🚀 performance Optimization, memory use, etc and removed enhancement New feature or request labels Nov 29, 2024
emilk added a commit that referenced this issue Dec 2, 2024
### What
* Closes #3135
* Proceeded by #7602
* Proceeded by #7603
* Proceeded by #8241
* New issue: #8249

This PR implements Undo and Redo for any edit to the active blueprint.


https://github.com/user-attachments/assets/05018729-f01e-42f4-a84f-b48dbf31b060

### Implementation
This implements undo/redo by editing the "time cursor" for the blueprint
timeline. Undo moves it backwards, redo forwards. When doing an action,
all redo history is erased from the store with a new
`ChunkStore::drop_time_range` function.

### Known shortcomings
* Undo doesn't work when the blueprint streams panel is open

### 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/7546?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/7546?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] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7546)
- [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`.

---------

Co-authored-by: Clement Rey <[email protected]>
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_datastore affects the datastore itself
Projects
None yet
Development

No branches or pull requests

1 participant