perf(scene): reserve EnTT storage + dedup AnimatedModel loads on deserialize#584
Conversation
…ene deserialize (#525) Two cheap wins for scene YAML load time, measured with the perf-stress battery (docs/guides/perf-stress-scenes.md): - Reserve the entity pool and the 3 always-added component storages (IDComponent/TransformComponent/TagComponent) up front in both Deserialize and DeserializeAdditive, using the known entity count from the YAML, so none of them reallocate mid-load. - Cache Ref<AnimatedModel> loads per resolved source path within a single SceneSerializer instance, so a crowd of entities sharing one rigged-character source file only pays Assimp import + GPU buffer allocation once instead of once per entity. Per-entity playback state (AnimationStateComponent) stays independent — verified by a new regression test (AnimatedModelDedupTest.cpp) that shares one glTF across two entities and confirms the model/skeleton Refs are shared while playback scalars are not. Scene-open-time results (anim_crowd/ecs_static perf-stress scenes): anim_crowd_5000 (5,003 entities): 56.9s -> 2.2s (~26x) anim_crowd_10000 (10,003 entities): >120s -> 4.3s (>28x) ecs_static_100000 (100,002 entities): >120s -> 20.0s (>6x) Full OloEngine-Tests suite (4198 tests) passes with no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughSceneSerializer now caches Ref<AnimatedModel> instances by source file path during entity deserialization to avoid redundant loads, threading the cache through DeserializeEntityComponents and DeserializeEntity. Scene deserialization also pre-reserves EnTT registry storage for large loads. A new integration test validates the dedup behavior. ChangesAnimated Model Dedup and Scene Loading
Sequence Diagram(s)sequenceDiagram
participant DeserializeEntity
participant DeserializeEntityComponents
participant AnimatedModelCache
participant AnimatedModel
DeserializeEntity->>DeserializeEntityComponents: pass m_AnimatedModelCache
DeserializeEntityComponents->>AnimatedModelCache: lookup by SourceFilePath
alt cached
AnimatedModelCache-->>DeserializeEntityComponents: return existing Ref
else not cached
DeserializeEntityComponents->>AnimatedModel: Create(SourceFilePath)
DeserializeEntityComponents->>AnimatedModelCache: insert new Ref
end
DeserializeEntityComponents-->>DeserializeEntity: entity with shared model, independent playback state
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|




Summary
Cheap-wins slice of #525 ("Scene: YAML load takes 24-59s at 50k-100k entities"), scoped to the two structural fixes identified without needing a binary scene format or runtime spawner rewrite:
SceneSerializer::DeserializeFromYAMLandDeserializeAdditivenow reserve the EnTT entity pool plus theIDComponent/TransformComponent/TagComponentstorages up front, using the known entity count from the YAMLEntitiessequence, instead of letting every pool reallocate repeatedly as entities stream in.SceneSerializernow cachesRef<AnimatedModel>per resolved absolute source path for the lifetime of oneDeserialize()/DeserializeFromYAML()call. A crowd of entities sharing one rigged-character source file (the common case for NPCs/enemies) now pays Assimp import + GPU-buffer allocation once instead of once per entity. Per-entity playback state (AnimationStateComponent) stays independent — verified by a new regression test.Results
Measured with the perf-stress battery (
docs/guides/perf-stress-scenes.md), scene-open time:anim_crowd_5000anim_crowd_10000ecs_static_100000ecs_static_100000has noAnimationStateComponentat all, isolating the reserve-only win; theanim_crowd_*scenes get both fixes stacked.Test plan
AnimatedModelDedupTest.cpp— two entities sharing one glTF source path share the sameMeshSource/SkeletonRef after deserialize, while independently-setAnimationStateComponentplayback scalars (m_CurrentTime,m_IsPlaying) survive un-aliased. Log evidence confirms only one Assimp import fires for the shared path.OloEngine-Testssuite (4198 tests) passes, 0 failures, 0 regressions.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests