decentraland / unity-explorer

Apache License 2.0
8 stars 11 forks source link

RoadInstantiatorSystem causes frame-rate spikes & Memory Allocations #1917

Open m3taphysics opened 2 weeks ago

m3taphysics commented 2 weeks ago

Explorer Alpha build versions:

image

Image

Issue Description:

RoadInstantiatorSystem allocates memory especially when running around and moving. This can lead to GC spikes and spikes on instantiation.

STR:

  1. Load genesis plaza
  2. Load the profiler and filter by "RoadInstantiatorSystem"
  3. Drop down the center and/or run around
  4. As you drop down you should notice framerate spikes and larger memory allocactions
  5. Expected behaviour:

Current behaviour:

Evidence:

Additional Notes:

QThund commented 2 weeks ago

Allocations occur when the object pool for a given road asset (there is a dictionary of asset<->pools) does not have enough cached objects and has to instantiate new ones. The only solutions seem to be either increasing the minimum cached objects or creating more than one object when there are not enough so we get the framerate spike less often. We could also try to release objects that are not in camera with the hope of reducing the chance of filling the pools, although this would surely produce popping; roads are unloaded as they are farther than a radius from the avatar, btw. Edit: Or another option could be to reduce the radius.

QThund commented 1 week ago

After studying the case, my conclussion is that the most reasonable solution is to cache N roads per pool and increase the amount of cached objects in a specific pool by X every time it gets full.

Pros:

Cons:

Example of the contents of the road asset pool (N=30, X=20):

Name: Total cached (being used)
Corner_0: 30 (15)
Corner_A: 30 (20)
Corner_B: 30 (15)
Corner_C: 30 (21)
Corner_D: 30 (17)
Crossroads_00: 30 (2)
Crossroads_0: 30 (4)
Crossroads_A: 30 (3)
Crossroads_C: 30 (5)
DeadEnd_0: 30 (2)
EmptyFork_0: 30 (2)
EmptyFork_B: 30 (3)
EmptyFork_D: 30 (1)
Fork_0: 30 (13)
Fork_A: 30 (16)
Fork_B: 30 (20)
Fork_C: 30 (13)
Fork_D: 30 (16)
OpenCorner_0: 30 (2)
OpenCorner_B: 30 (2)
OpenCorner_C: 30 (1)
OpenFork_0: 30 (5)
OpenFork_A: 30 (8)
OpenFork_B: 30 (3)
OpenFork_C: 30 (1)
OpenRoad_0: 30 (30)
OpenRoad_A: 50 (31)
OpenRoad_B: 30 (28)
OpenRoad_C: 50 (31)
OpenRoad_D: 50 (31)
Road_0: 130 (112)
Road_A: 150 (137)
Road_B: 70 (66)
Road_C: 110 (97)
Road_D: 110 (98)
Road_E: 50 (50)
DeadEnd_A: 30 (1)
DeadEnd_C: 30 (3)
DeadEnd_E: 30 (2)
OpenCorner_A: 30 (2)
-49,-51: 30 (0)
-51,-49: 30 (1)
-70,-1: 30 (1)
0,-69: 30 (1)
0,12: 30 (1)
2,-51: 30 (0)
2,50: 30 (0)
13,0: 30 (1)

Additional thoughts: We could add more diversity to the most used road assets so there are more small pools and hence less memory allocations after loading the scene. For example, if Road_A requires 137 instances, there could be a Road_A1, Road_A2, Road_A3 and Road_A4 in the map, so pools could store 30, 30, 30, 30, 17.

As a real example, currently the extra memory we would need, using N=30 and X=20, would be 3Gb.

QThund commented 1 week ago

After a lot of testing and implementing different solutions (which include releasing road instances that were behind the camera), it seems that the most balanced solution so far is this:

The extra memory required for this solution is around 200 Mb. The amount of moments when FPS are affected by road instantiation is way lower (although could be more noticeable), and may occur during the first minute mostly.

A video just to illustrate popping: https://github.com/user-attachments/assets/b5cac978-04b5-4e2c-bd9f-f8cdd2372af0