Repository navigation
perf(render): deserialize a scene's children once instead of once per frame - #419
Merged
Merged
Conversation
… frame prepare_scene ran on the per-frame path and had no cache, although its result depends only on the scene. Every frame cloned the whole serde_json subtree of every child and deserialized it into ChildComponent -- an untagged enum with 57 variants, so serde re-parses the same object once per failed variant before it lands. Measured on two scenarios, timing the call for every frame of a render: mega-showcase, 1236 frames, 9 scenes 144ms -> 46ms 50 nodes in one scene, 1200 frames 388ms -> 3.2ms The 46ms that remain on mega-showcase are the nine first-time deserializations, which is the work that actually has to happen. End-to-end this is within noise on both (9.0s and 8.5s): the redundancy was 1.6% and 4.5% of a render, spread across workers. The issue asked for the order of magnitude before deciding how much to spend, and this is it -- so the fix is the contained one, not the restructuring. The memo is a OnceLock on the Scene rather than a keyed cache, so it has no identity problem and no invalidation to get wrong: it dies with the scenario. rustmotion-core cannot name ChildComponent, which lives a crate above it, so the slot holds Arc<dyn Any + Send + Sync> and prepare_scene is the typed accessor -- and its only writer, which is what makes the downcast total. The studio rebuilds a whole ResolvedScenario on every optimistic edit and swaps the Arc, so its scenes are new objects with fresh slots; there is no path that mutates scene.children in place after load. deserialize_children's stderr warning for an unreadable child rode on the repetition. It is now emitted once per scene.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #159.
Measured first, because the issue asked for that
prepare_sceneran on the per-frame path with no cache, although its resultdepends only on the scene. Every frame cloned the whole
serde_jsonsubtree ofevery child and deserialized it into
ChildComponent— an untagged enum with 57variants, so serde re-parses the same object once per failed variant before it
lands.
Timing the call for every frame of a render, release build:
examples/mega-showcase.json— 1236 frames, 9 scenes, 31 childrenThe 46 ms that remain on mega-showcase are the nine first-time deserializations —
the work that actually has to happen.
End to end this is within noise on both (9.0 s and 8.5 s): the redundancy was
1.6 % and 4.5 % of a render, spread across workers. That is the order of magnitude
the issue asked for before deciding how much to spend, and it is why this is the
contained fix rather than the restructuring the issue sketched. It grows with node
count, which is where it will matter.
Shape
The memo is a
OnceLockon theScene, not a keyed cache. So it has noidentity problem and no invalidation to get wrong: it dies with the scenario.
rustmotion-corecannot nameChildComponent, which lives a crate above it, sothe slot holds
Arc<dyn Any + Send + Sync>andprepare_sceneis the typedaccessor — and its only writer, which is what makes the downcast total.
Checked before relying on it: the studio rebuilds a whole
ResolvedScenarioonevery optimistic edit and swaps the
Arc, so its scenes are new objects withfresh slots.
include.rs's&mut Sceneruns at load, before any render. No pathmutates
scene.childrenin place afterwards.The warning the issue mentions
deserialize_childrenemits a stderr line per unreadable child. That fired onceper frame, riding on the same repetition. It is now once per scene, pinned by
a_child_that_cannot_be_read_is_reported_once_rather_than_once_per_frame.Tests
Four, on
Arc::ptr_eq— which is the direct statement of "deserialized once".Reverting to an uncached
Arc::new(deserialize_children(scene))fails two of them.Gate
cargo fmt --all --checkclean ·cargo clippy --workspace --all-targets --features rustmotion/studio -D warningsclean ·cargo test --workspace1802 passed.