decentraland / decentraland/sdk
Reduce component's model allocations
- Dominant language
- No language data
- Stars
- 4
- Forks
- 5
- PR merge metrics
- No merged PRs in 30d
Description
Epic: https://github.com/decentraland/sdk/issues/895
Every time a component's crdt is received, a instance of it's data is instantiated, generating a new allocation.
This behavior can be problematic. In an extreme scenario it could crash the client due to memory fragmentation, In common scenario it will generate hiccups related to garbage collection.
The proposed solution is to add a pool for the components models in `ECSComponent` class, where we get an already instantiated model when component is created for an entity and release that model when component is removed, while we just update that instance when component is updated.
We already got a pooling class used for sending components data from the renderer to the scene.
https://github.com/decentraland/unity-renderer/blob/dev/unity-renderer/Assets/DCLPlugins/ECS7/ComponentWrapper/Generic/WrappedComponentPool.cs
used here, as an example: https://github.com/decentraland/unity-renderer/blob/dev/unity-renderer/Assets/DCLPlugins/ECS7/Systems/PlayerSystem/ECSPlayerTransformSystem.cs#L77-L80
We also got code for updating a model from a byte span without needing to alloc a new instance
https://github.com/decentraland/unity-renderer/blob/dev/unity-renderer/Assets/DCLPlugins/ECS7/ComponentWrapper/ProtobufWrappedComponent.cs#L29-L33
The main problem we' ll face while changing this implementation is that `ECSComponent` is used both by scene components and internal components.
scene components `ModelType` is a serializable/deserializabe `class` (Protobuf components and Transform component)
while internal components `ModelType` is a `struct` that is not serializable/deserializable and **it shouldn't be**
So objects pooling in `ECSComponent` should only apply to scene's component and not internal components.
Solving this problem with inheritance is totally discouraged and we should only do it as our last resort, we should try to inject all those implementation that branch from different components types through `ECSComponent` constructor like `public ECSComponent(IImplementation impl, Func> handlerBuilder)`
the lambda `Func deserializer` that is currently injected on that constructor won't be needed any more after this changes in the implementation are done
Contributor guide
Assessment
This issue has not been assessed yet.