-
Notifications
You must be signed in to change notification settings - Fork 373
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
GC the blueprints before saving while preserving the current state #3148
Conversation
…nating the profile
6eead83
to
314bc83
Compare
7c9ae50
to
2c4a3ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks really good, but there was one piece of logic I failed to follow.
There is also, I believe, a chance to reduce code duplication by recognizing that "Everything" can be expressed numerically
@@ -13,6 +16,31 @@ pub enum GarbageCollectionTarget { | |||
/// | |||
/// The fraction must be a float in the range [0.0 : 1.0]. | |||
DropAtLeastFraction(f64), | |||
|
|||
/// GC Everything that isn't protected | |||
Everything, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this different from DropAtLeastFraction(1.0)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two differences:
- DropAtLeastFraction does size book-keeping and potentially decides to stop early. I was running into edge cases where it would conclude it had met the 1.0 threshold, but in fact still had stuff it could GC.
- DropAtLeastFraction needs to do an oldest-first incremental traversal. Everything has lots more room for optimizations that require less shuffling and re-sorting.
Special-casing the numerical value of 1.0 felt weirder to me than special-casing on an explicit enum but I can make that switch if you think it's clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that makes sense 👍
…3148) Resolves: #3098 Related to: #1803 Because blueprints used timeless data and timeless data wasn't GC'd, we previously had no great way to clean up blueprints. This PR paves the way for better overall GC behavior in the future but doesn't change the default behavior yet. This PR: - Introduces a new `GarbageCollectionOptions` instead of just providing a target. This allows you to configure whether you want to gc the timeless data, and additionally how many latest_at values you want to preserve. - Introduces a new gc target: Everything. - Calculates a set of protected rows for every component based on the last relevant row across every timeline (including timeless). - Modifies both `gc_drop_at_least_num_bytes` and the new `gc_everything` to respect the protected rows during gc. - Modifies the store_hub to gc the blueprint before saving it. Photogrammetry with `--no-frames` is another "worst-case" for blueprint because every image is a space-view, so you can easily create a huge blueprint history by repeatedly resetting the blueprint. ![image](https://github.com/rerun-io/rerun/assets/3312232/03df3d06-a780-47b3-b0d9-aaf564793230) * [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/3148) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3148) - [Docs preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/docs) - [Examples preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/examples) - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
…3148) Resolves: #3098 Related to: #1803 Because blueprints used timeless data and timeless data wasn't GC'd, we previously had no great way to clean up blueprints. This PR paves the way for better overall GC behavior in the future but doesn't change the default behavior yet. This PR: - Introduces a new `GarbageCollectionOptions` instead of just providing a target. This allows you to configure whether you want to gc the timeless data, and additionally how many latest_at values you want to preserve. - Introduces a new gc target: Everything. - Calculates a set of protected rows for every component based on the last relevant row across every timeline (including timeless). - Modifies both `gc_drop_at_least_num_bytes` and the new `gc_everything` to respect the protected rows during gc. - Modifies the store_hub to gc the blueprint before saving it. Photogrammetry with `--no-frames` is another "worst-case" for blueprint because every image is a space-view, so you can easily create a huge blueprint history by repeatedly resetting the blueprint. ![image](https://github.com/rerun-io/rerun/assets/3312232/03df3d06-a780-47b3-b0d9-aaf564793230) * [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/3148) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3148) - [Docs preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/docs) - [Examples preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/examples) - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
…3148) Resolves: #3098 Related to: #1803 Because blueprints used timeless data and timeless data wasn't GC'd, we previously had no great way to clean up blueprints. This PR paves the way for better overall GC behavior in the future but doesn't change the default behavior yet. This PR: - Introduces a new `GarbageCollectionOptions` instead of just providing a target. This allows you to configure whether you want to gc the timeless data, and additionally how many latest_at values you want to preserve. - Introduces a new gc target: Everything. - Calculates a set of protected rows for every component based on the last relevant row across every timeline (including timeless). - Modifies both `gc_drop_at_least_num_bytes` and the new `gc_everything` to respect the protected rows during gc. - Modifies the store_hub to gc the blueprint before saving it. Photogrammetry with `--no-frames` is another "worst-case" for blueprint because every image is a space-view, so you can easily create a huge blueprint history by repeatedly resetting the blueprint. ![image](https://github.com/rerun-io/rerun/assets/3312232/03df3d06-a780-47b3-b0d9-aaf564793230) * [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/3148) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3148) - [Docs preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/docs) - [Examples preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/examples) - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
…3148) Resolves: #3098 Related to: #1803 Because blueprints used timeless data and timeless data wasn't GC'd, we previously had no great way to clean up blueprints. This PR paves the way for better overall GC behavior in the future but doesn't change the default behavior yet. This PR: - Introduces a new `GarbageCollectionOptions` instead of just providing a target. This allows you to configure whether you want to gc the timeless data, and additionally how many latest_at values you want to preserve. - Introduces a new gc target: Everything. - Calculates a set of protected rows for every component based on the last relevant row across every timeline (including timeless). - Modifies both `gc_drop_at_least_num_bytes` and the new `gc_everything` to respect the protected rows during gc. - Modifies the store_hub to gc the blueprint before saving it. Photogrammetry with `--no-frames` is another "worst-case" for blueprint because every image is a space-view, so you can easily create a huge blueprint history by repeatedly resetting the blueprint. ![image](https://github.com/rerun-io/rerun/assets/3312232/03df3d06-a780-47b3-b0d9-aaf564793230) * [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/3148) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3148) - [Docs preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/docs) - [Examples preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/examples) - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
…3148) Resolves: #3098 Related to: #1803 Because blueprints used timeless data and timeless data wasn't GC'd, we previously had no great way to clean up blueprints. This PR paves the way for better overall GC behavior in the future but doesn't change the default behavior yet. This PR: - Introduces a new `GarbageCollectionOptions` instead of just providing a target. This allows you to configure whether you want to gc the timeless data, and additionally how many latest_at values you want to preserve. - Introduces a new gc target: Everything. - Calculates a set of protected rows for every component based on the last relevant row across every timeline (including timeless). - Modifies both `gc_drop_at_least_num_bytes` and the new `gc_everything` to respect the protected rows during gc. - Modifies the store_hub to gc the blueprint before saving it. Photogrammetry with `--no-frames` is another "worst-case" for blueprint because every image is a space-view, so you can easily create a huge blueprint history by repeatedly resetting the blueprint. ![image](https://github.com/rerun-io/rerun/assets/3312232/03df3d06-a780-47b3-b0d9-aaf564793230) * [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/3148) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3148) - [Docs preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/docs) - [Examples preview](https://rerun.io/preview/60f3747383780c50886ac781bdf81b32fbff76bd/examples) - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
What
Resolves: #3098
Related to: #1803
Because blueprints used timeless data and timeless data wasn't GC'd, we previously had no great way to clean up blueprints.
This PR paves the way for better overall GC behavior in the future but doesn't change the default behavior yet.
This PR:
GarbageCollectionOptions
instead of just providing a target. This allows you to configure whether you want to gc the timeless data, and additionally how many latest_at values you want to preserve.gc_drop_at_least_num_bytes
and the newgc_everything
to respect the protected rows during gc.Photogrammetry with
--no-frames
is another "worst-case" for blueprint because every image is a space-view, so you can easily create a huge blueprint history by repeatedly resetting the blueprint.Checklist