Question Advice on optimising this script?

I have this script that controls the behaviour of props in the game. For instance if a lamppost is hit with force it falls over and then disappears after few seconds by switching off the renderer. Once the player gets far enough away the lamppost renderer will be switched on again. This is to hide the pop in of props being visible again. It also stores the original position of the prop so even after falling over it will reset back to its original position. However using the profiler it routinely is highest on the list affecting performance. I tried to delay the update to not call it every frame. Just trying to improve performance.

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.  
5. public class ResetTransform : MonoBehaviour
6. {
7.  
8.     Vector3 originalPos;
9.     Quaternion originalRot;
10.  
11.     private Rigidbody rb;
12.     private Renderer rend;
13.  
14.     float x;
15.     float y;
16.     float z;
17.  
18.     public float collisionforce;
19.     public GameObject Player;
20.     public List<GameObject> ParticleObject;
21.     public float Distance_;
22.  
23.     private float timeSinceLastCalled;
24.     private float delay = 60f;
25.  
26.  
27.     // Start is called before the first frame update
28.     void Start()
29.     {
30.  
31.         originalPos = new Vector3(gameObject.transform.position.x, gameObject.transform.position.y, gameObject.transform.position.z);
32.         originalRot = Quaternion.Euler(gameObject.transform.rotation.eulerAngles.x, gameObject.transform.rotation.eulerAngles.y, gameObject.transform.rotation.eulerAngles.z);
33.  
34.         rb = GetComponent<Rigidbody>();
35.         rend = GetComponent<Renderer>();
36.  
37.         Player = GameObject.FindGameObjectWithTag("MainCamera");
38.  
39.  
40.  
41.     }
42.  
43.     void Update()
44.     {
45.  
46.         Distance_ = Vector3.Distance(this.gameObject.transform.position, Player.transform.position);
47.         timeSinceLastCalled += Time.deltaTime;
48.         if (Distance_ > 150 && timeSinceLastCalled>delay)
49.  
50.         {
51.  
52.             Collider[] myColliders = gameObject.GetComponents<Collider>();
53.             rend.enabled = true;
54.  
55.             foreach (Transform child in transform)
56.                 child.gameObject.SetActive(true);
57.  
58.             foreach (Collider bc in myColliders) bc.enabled = true;
59.             timeSinceLastCalled = 0f;
60.  
61.         }
62.  
63.  
64.     }
65.  
66.     // Update is called once per frame
67.     void OnCollisionEnter(Collision other)
68.     {
69.  
70.  
71.         if (other.gameObject.tag == "IaCar" && (other.relativeVelocity.magnitude > collisionforce))
72.  
73.         {
74.  
75.             rb.isKinematic = false;
76.             foreach (GameObject obj in ParticleObject) obj.SetActive(true);
77.             StartCoroutine(ResetObjectCoroutine());
78.  
79.  
80.         }
81.  
82.  
83.         IEnumerator ResetObjectCoroutine()
84.         {
85.  
86.             //yield on a new YieldInstruction that waits for 5 seconds.
87.             yield return new WaitForSeconds(10);
88.  
89.             rend.enabled = false;
90.  
91.             foreach (Transform child in transform)
92.                 child.gameObject.SetActive(false);
93.  
94.             Collider[] myColliders = gameObject.GetComponents<Collider>();
95.             foreach (Collider bc in myColliders) bc.enabled = false;
96.  
97.  
98.             transform.rotation = originalRot;
99.             gameObject.transform.position = originalPos;
100.             rb.isKinematic = true;
101.             foreach (GameObject obj in ParticleObject) obj.SetActive(false);
102.  
103.         }
104.  
105.     }
106. }
107.  

 

Have you turned on the "deep" profiler to look into what parts exactly about it are the highest?

Got a screenshot of those results?

I'm off for date night with the wifey, but that's the first thing I'd want to see before spending time combing through your code.

...

Little fast one off things I'm seeing.

1) in your Update every 'delay' when distance is far, you're creating a 'Collider' array with that GetComponents. Maybe cache that array on Awake?

2) you have magnitude tests, make them sqrMagnitudes (not a huge change, but hey, still saves you a sqrt)

3) you spin up a Coroutine in your OnCollisionEnter that also creates some garbage with the waitforseconds. Also if collisionenter occurs again during that 10 seconds you can get some doubling up. Add some safety checks to avoid doubling up and maybe switch to something like UniTask to lighten that load.

4) Use CompareTag, not tag ==

5) Why is Distance_ a public field (serialized?) when its updated during Update. Weird name is also weird. Why not make this a local variable to the Update method?

...

With all that said... none of this is like terrifying large amounts of issues. I didn't do a deep analysis of your script, just a once over, but still... nothing scary here. Just sloppy.

So I again go back to what is your profiler actually saying? What performance hit is it actually having?

 

This script doesn't do a whole lot. How many instances are there at the same time? How much actual time does the profiler report them taking per frame? Also you should be able to drill-down further and find which part of the script is taking the most time.

Edit: If you have something like 1000 or more and it's you distance check that's taking up all that time, AND that time is actually impacting your frame-rate then my advice would be to modify the script so instead of one script on each object, divide up your map into zones and have one script for the whole zone. Then when the player is far enough away have the script reset multiple objects- everything in the zone that needs to be reset.

 

So if you see below the reset transform update function it highest. It is attached to all dynamic props that will be knocked over etc The performance hovers between 40fps-55fps doesn't sound bad but working on medium sized city level with dozens of props so wish to optimise each area. Enjoy your date!



Deep profiling.



will try some of these suggestions posted.

 

Looking at the profiler 978? city type level with interactable props that can be knocked over etc.

 

I suspect this line:

Code (CSharp):

1. foreach (Transform child in transform)
2.                 child.gameObject.SetActive(true);

Activating game objects can cause a lot of transform update events to fire. It will be faster in any case if you were to activate a single object that contains all the children rather than foreaching over the children yourself.

And then it will matter how deep and complex the hierarchy of these objects are. If most of them have hundreds of child and child of child etc objects then that‘s going to cost a little for every child and if there are hundreds or more it can add up. More so if they also have scripts that perform stuff in OnEnable, or do something in their first update. Typically it‘s faster to have a single script control all children rather than each child running a script of their own.

 

Maybe the big one is in line 46,47,48: don't compute the distance for the 95% of the time you won't look at it. Put a timeSinceLastCalled IF first, and only if that passes compute the distance. I think coroutines are even faster at waiting -- a loop with yield return new WaitForSeconds(2.0f).

 

Right, though this is actually micro-optimization given how little Vector3.Distance contributes to the Update method's timings.

Tackle the topmost items first, which would be:

• replace this.gameObject.transform with just transform to avoid the unnecessary property call (that's GameObject.get_transform() in the profiler)
• cache Player.transform in a field to avoid the unnecessary property call (that's Component.get_transform() in the profiler)
• learn the lesson that properties are in fact method calls, and method calls do come with a minimal overhead that can accumulate to significant times when called often enough
• and then comes the almost negligible part, optimizing the actual calculation:
  • compute the square distance only (avoid taking square root)
  • compare the result with 150*150 (squared distance)
  • do the same but use Unity.Mathematics methods and types for calculations and fields
  • rename Distance_ to Distance because I find trailing underscores abhorrent

 

I'd say both of y'all are correct.

Owen's "micro-optimization" impacts every frame giving a better average compute time. Optimizing the overall average framerate.

CodeSmile's optimizes the frame where the work actually occurs. Optimizing the risk of "frame drop/stutter".

I ran some tests to get my own numbers on my machine. I filled a scene with a grid of cubes with OP's script on it like so. With all the necessary colliders/renderer/etc and set "delay" to 1 to have the hard work occur more frequently:

(1000 - close to what OP said they had)

I ran it as is and got these averages:

Code (csharp):

1. DOWN FRAMES | HARD FRAMES | AVG FPS
2. 2.5ms       | 7.75ms         | 40-51fps     (as-is)
3. 0.22ms      | 7.75ms         | 51-61fps     (no dist check until needed)
4. 0.22ms      | 3.2ms          | 51-61fps     (removed the foreach transform alltogether)
5. 0.22ms      | 2.33ms         | 51-61fps     (removed foreach and cached collider array)

The 4 updates looked as follows:

Spoiler

1)

Code (csharp):

1.         void Update()
2.         {
3.             Distance_ = Vector3.Distance(this.gameObject.transform.position, Player.transform.position);
4.             timeSinceLastCalled += Time.deltaTime;
5.             if (Distance_ > 150 && timeSinceLastCalled > delay)
6.  
7.             {
8.                 Collider[] myColliders = gameObject.GetComponents<Collider>();
9.                 rend.enabled = true;
10.  
11.                 foreach (Transform child in transform)
12.                     child.gameObject.SetActive(true);
13.  
14.                 foreach (Collider bc in myColliders) bc.enabled = true;
15.                 timeSinceLastCalled = 0f;
16.             }
17.         }

2)

Code (csharp):

1.         void Update()
2.         {
3.             timeSinceLastCalled += Time.deltaTime;
4.             if (timeSinceLastCalled < delay) return;
5.             timeSinceLastCalled = 0f;
6.  
7.             Distance_ = Vector3.Distance(this.gameObject.transform.position, Player.transform.position);
8.             if (Distance_ > 150)
9.             {
10.                 Collider[] myColliders = gameObject.GetComponents<Collider>();
11.                 rend.enabled = true;
12.  
13.                 foreach (Transform child in transform)
14.                     child.gameObject.SetActive(true);
15.  
16.                 foreach (Collider bc in myColliders) bc.enabled = true;
17.             }
18.         }

3)

Code (csharp):

1.         void Update()
2.         {
3.             timeSinceLastCalled += Time.deltaTime;
4.             if (timeSinceLastCalled < delay) return;
5.             timeSinceLastCalled = 0f;
6.  
7.             Distance_ = Vector3.Distance(this.gameObject.transform.position, Player.transform.position);
8.             if (Distance_ > 150)
9.             {
10.                 Collider[] myColliders = gameObject.GetComponents<Collider>();
11.                 rend.enabled = true;
12.  
13.                 foreach (Collider bc in myColliders) bc.enabled = true;
14.             }
15.         }

4)

Code (csharp):

1. //note that m_colliders is cached in Start
2.         void Update()
3.         {
4.             timeSinceLastCalled += Time.deltaTime;
5.             if (timeSinceLastCalled < delay) return;
6.             timeSinceLastCalled = 0f;
7.  
8.             Distance_ = Vector3.Distance(this.gameObject.transform.position, Player.transform.position);
9.             if (Distance_ > 150)
10.             {
11.                 rend.enabled = true;
12.  
13.                 foreach (Collider bc in m_colliders) bc.enabled = true;
14.             }
15.         }

Note that the average fps doesn't actually change when we optimize/remove the stuff in the if statement as this only happens once per 'delay' (in the case of OPs original it happens once per minute). So effectively it's a frame drop we're avoiding.

But Owen's optimization saved us a lot on the moment to moment (10 whole fps).

...

I'll be back in a bit with my full optimization tip (I'm in the midst of a few things right now, so it may be a moment).

My optimizations would likely include:
1) Owen's and SmileCode's ideas as they are not wrong at all
2) Moving this to a single Update to avoid that overhead
3) staggering the work across multiple frames in clusters of objects
4) various micro-optimizations

 

So I'm back... had some meetings to do with a buddy.

My first pass results in similar behaviour to OP where we disable every child and colliders on the object. Maybe OP requires this behaviour. Maybe the layout of the object can't have a container transform to disable, or it can't use itself because other scripts attached on the root object still need to run.

NOTE - I removed the OnCollisionEnter portion as that is not part of our optimization tests here. That can be tackled independently of this.

Code (csharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4. using Cysharp.Threading.Tasks;
5.  
6. namespace SpacepuppyExampleScripts
7. {
8.  
9.     public class zTest02 : MonoBehaviour
10.     {
11.  
12.         Vector3 originalPos;
13.         Quaternion originalRot;
14.  
15.         private Rigidbody rb;
16.         private Renderer rend;
17.  
18.         float x;
19.         float y;
20.         float z;
21.  
22.         public float collisionforce;
23.         public GameObject Player;
24.         public List<GameObject> ParticleObject;
25.  
26.         private Collider[] m_colliders;
27.  
28.         // Start is called before the first frame update
29.         void Start()
30.         {
31.             originalPos = new Vector3(gameObject.transform.position.x, gameObject.transform.position.y, gameObject.transform.position.z);
32.             originalRot = Quaternion.Euler(gameObject.transform.rotation.eulerAngles.x, gameObject.transform.rotation.eulerAngles.y, gameObject.transform.rotation.eulerAngles.z);
33.  
34.             rb = GetComponent<Rigidbody>();
35.             rend = GetComponent<Renderer>();
36.             m_colliders = GetComponents<Collider>();
37.         }
38.  
39.         private void OnEnable()
40.         {
41.             _pool.AddMember(this);
42.         }
43.  
44.         private void OnDisable()
45.         {
46.             _pool.RemoveMember(this);
47.         }
48.  
49.         private static readonly WorkPool _pool = new WorkPool();
50.         private class WorkPool
51.         {
52.  
53.             private HashSet<zTest02> members = new HashSet<zTest02>();
54.  
55.             const float DELAY = 1f;
56.             const int SPREAD = 10;
57.             private int version;
58.             private int loopversion;
59.  
60.             public void AddMember(zTest02 obj)
61.             {
62.                 if (members.Add(obj))
63.                 {
64.                     loopversion++;
65.                     if (members.Count == 1)
66.                     {
67.                         version++;
68.                         _ = DoWork(version);
69.                     }
70.                 }
71.             }
72.  
73.             public void RemoveMember(zTest02 obj)
74.             {
75.                 if (members.Remove(obj) && members.Count == 0)
76.                 {
77.                     version++;
78.                 }
79.             }
80.  
81.  
82.             async UniTask DoWork(int v)
83.             {
84.                 await UniTask.Yield();
85.  
86.                 while (v == version)
87.                 {
88.                     var cam = Camera.main; //this already references the object tagged 'MainCamera'
89.  
90.                     //spread across multiple frames
91.                     var e = members.GetEnumerator();
92.                     int lv = loopversion;
93.                     int max = System.Math.Max(1, Mathf.CeilToInt((float)members.Count / SPREAD));
94.                     for (int i = 0; i < SPREAD; i++)
95.                     {
96.                         for (int j = 0; j < max; j++)
97.                         {
98.                             if (lv != loopversion || !e.MoveNext()) goto CompletedShort;
99.  
100.                             var obj = e.Current;
101.                             float dist = (obj.transform.position - cam.transform.position).sqrMagnitude;
102.                             if (dist > (150 * 150))
103.                             {
104.                                 obj.rend.enabled = true;
105.  
106.                                 foreach (Transform child in obj.transform)
107.                                     child.gameObject.SetActive(true);
108.  
109.                                 foreach (Collider bc in obj.m_colliders) bc.enabled = true;
110.                             }
111.                         }
112.                         await UniTask.Yield();
113.                     }
114.                 CompletedShort:
115.                     await UniTask.Delay(System.TimeSpan.FromSeconds(DELAY));
116.                 }
117.             }
118.  
119.         }
120.  
121.     }
122.  
123. }

In this our moment to moment is basically NOTHING as we just allow UniTask to wait out a single timer of DELAY length. The amount of performance doesn't even show up in my profiler.
And when our time hits to do our work it spreads it across SPREAD frames, or in my case 10 frames, I got an average cost of 1.2ms per frame.

So that's:
0ms | 1.2ms | 60-73fps

But... lets just say that isn't the case and they can optimize the hierarchy. So this way goes with CodeSmile's choice of using 'transform' (self) to enable disable everything. We need to move our logic around on Awake/OnDestroy, but that's ok. This one will include the OnCollisionEnter as it too would need major changes as well to match the logic. This would also hint at ways we can optimize that one (the use of unitask).

Code (csharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4. using Cysharp.Threading.Tasks;
5.  
6. namespace SpacepuppyExampleScripts
7. {
8.  
9.     public class zTest02 : MonoBehaviour
10.     {
11.  
12.         Vector3 originalPos;
13.         Quaternion originalRot;
14.  
15.         private Rigidbody rb;
16.         private Renderer rend;
17.  
18.         float x;
19.         float y;
20.         float z;
21.  
22.         public float collisionforce;
23.         public GameObject Player;
24.         public List<GameObject> ParticleObject;
25.  
26.         // Start is called before the first frame update
27.         void Awake()
28.         {
29.             originalPos = new Vector3(gameObject.transform.position.x, gameObject.transform.position.y, gameObject.transform.position.z);
30.             originalRot = Quaternion.Euler(gameObject.transform.rotation.eulerAngles.x, gameObject.transform.rotation.eulerAngles.y, gameObject.transform.rotation.eulerAngles.z);
31.  
32.             rb = GetComponent<Rigidbody>();
33.             rend = GetComponent<Renderer>();
34.  
35.             _pool.AddMember(this);
36.         }
37.  
38.         private void OnDestroy()
39.         {
40.             _pool.RemoveMember(this);
41.         }
42.  
43.         void OnCollisionEnter(Collision other)
44.         {
45.             if (other.gameObject.CompareTag("IaCar") && (other.relativeVelocity.sqrMagnitude > (collisionforce * collisionforce)))
46.             {
47.                 rb.isKinematic = false;
48.                 foreach (GameObject obj in ParticleObject) obj.SetActive(true);
49.                 _ = DisableAfterDelay();
50.             }
51.  
52.             async UniTask DisableAfterDelay()
53.             {
54.                 await UniTask.Delay(System.TimeSpan.FromSeconds(10f));
55.  
56.                 this.gameObject.SetActive(false);
57.             }
58.         }
59.  
60.  
61.         private static readonly WorkPool _pool = new WorkPool();
62.         private class WorkPool
63.         {
64.  
65.             private HashSet<zTest02> members = new HashSet<zTest02>();
66.  
67.             const float DELAY = 1f;
68.             const int SPREAD = 10;
69.             private int version;
70.             private int loopversion;
71.  
72.             public void AddMember(zTest02 obj)
73.             {
74.                 if (members.Add(obj))
75.                 {
76.                     loopversion++;
77.                     if (members.Count == 1)
78.                     {
79.                         version++;
80.                         _ = DoWork(version);
81.                     }
82.                 }
83.             }
84.  
85.             public void RemoveMember(zTest02 obj)
86.             {
87.                 if (members.Remove(obj) && members.Count == 0)
88.                 {
89.                     version++;
90.                 }
91.             }
92.  
93.  
94.             async UniTask DoWork(int v)
95.             {
96.                 await UniTask.Yield();
97.  
98.                 while (v == version)
99.                 {
100.                     var cam = Camera.main; //this already references the object tagged 'MainCamera'
101.  
102.                     //spread across multiple frames
103.                     var e = members.GetEnumerator();
104.                     int lv = loopversion;
105.                     int max = System.Math.Max(1, Mathf.CeilToInt((float)members.Count / SPREAD));
106.                     for (int i = 0; i < SPREAD; i++)
107.                     {
108.                         for (int j = 0; j < max; j++)
109.                         {
110.                             if (lv != loopversion || !e.MoveNext()) goto CompletedShort;
111.  
112.                             var obj = e.Current;
113.                             if (obj.gameObject.activeSelf) continue;
114.  
115.                             float dist = (obj.transform.position - cam.transform.position).sqrMagnitude;
116.                             if (dist > (150 * 150))
117.                             {
118.                                 obj.gameObject.SetActive(true);
119.                             }
120.                         }
121.                         await UniTask.Yield();
122.                     }
123.                 CompletedShort:
124.                     await UniTask.Delay(System.TimeSpan.FromSeconds(DELAY));
125.                 }
126.             }
127.  
128.         }
129.  
130.     }
131.  
132. }

This yielded:
0ms | 0.42ms | 61-73fps

Note that this actually didn't increase our framerate much. Cause again, the optimizing of the transform itself is the in the moment work. This just smooths out frame drops/stutters.

Note this could be accomplished without UniTask, could just use a global Coroutine for instance. I just find UniTask convenient.

...

And this was just using a single update.

Could look into potentially using something like jobs?

But I don't know... I've tossed enough at this.

 

So I'm sitting here eating lunch, and thoughts are going through my head as they do, and I realized... your code doesn't really matter until after that OnCollisionEnter event occurs (that's when the objects all get disabled).

A super fast way to optimize is to add it to this pool on that event OR if you don't even like the pool, spin up a coroutine then.

 

Wasn't expecting such great answers! Little tweaks recommend by yourself and @lordofduct edits the script doesn't even appear at the top of the profiler anymore! I didn't get the negligible part but did rename the Distance_ lol

 

This was really insightful thank you the script now has gone down the list on the profiler. I did need to disable the children as it holds the LOD objects and colliders. Colliders are switched on again when the player goes far enough so the object can get knocked over again. Your edits and @CodeSmile recommendations were valuable. The script now has gone down the list on the profiler

 

I had a feeling just disabling a parent gameobject might not be effective for you.

 

Are you sure? I haven't done exactly what you're doing, but I've disabled plenty of parents who had children with colliders. The child colliders (and renderers, of course) were auto-disabled as well. I've never put scripts on those children, so only assume those are turned off as well.

Way, way back Unity didn't have that rule -- you had to recursively disable/enable all children. But that was several versions ago.