c# why is this code be causing so much Garbage collection ?

why is this code be causing so much Garbage collection ?

I would appreciate any help you are willing to give in this matter:

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class DestoryOverTime : MonoBehaviour {
5.  
6.     public GameObject SpawnedObject;
7.     public float ReturnTime = 25;
8.     public bool poolAfterComplete = true;
9.    
10.     void Update ()
11.     {
12.         if (SpawnedObject) {
13.        
14.             Invoke ("ResetEffect", ReturnTime);
15.         }
16.     }
17.  
18.     public virtual void ResetEffect ()
19.     {
20.         if(poolAfterComplete)
21.         {
22.             NewObjectPool.instance.PoolObject(SpawnedObject);
23.  
24.         } else {
25.  
26.             Destroy(SpawnedObject);
27.         }
28.     }
29. }
30.  

The Object Pooler Code, just in case:

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. public class NewObjectPool : MonoBehaviour {
6.  
7.     /// <summary>
8.     /// Repository of commonly used prefabs.
9.     /// </summary>
10.     [AddComponentMenu("Gameplay/ObjectPool")]
11.     public static NewObjectPool instance { get; private set; }
12.    
13.     #region member
14.     /// <summary>
15.     /// Member class for a prefab entered into the object pool
16.     /// </summary>
17.     [System.Serializable]
18.     public class ObjectPoolEntry {
19.         /// <summary>
20.         /// the object to pre instantiate
21.         /// </summary>
22.         [SerializeField]
23.         public GameObject Prefab;
24.        
25.         /// <summary>
26.         /// quantity of object to pre-instantiate
27.         /// </summary>
28.         [SerializeField]
29.         public int Count;
30.     }
31.     #endregion
32.    
33.     /// <summary>
34.     /// The object prefabs which the pool can handle
35.     /// by The amount of objects of each type to buffer.
36.     /// </summary>
37.     public ObjectPoolEntry[] Entries;
38.    
39.     /// <summary>
40.     /// The pooled objects currently available.
41.     /// Indexed by the index of the objectPrefabs
42.     /// </summary>
43.     [HideInInspector]
44.     public List<GameObject>[] Pool;
45.    
46.     /// <summary>
47.     /// The container object that we will keep unused pooled objects so we dont clog up the editor with objects.
48.     /// </summary>
49.     protected GameObject ContainerObject;
50.    
51.     void OnEnable()
52.     {
53.         instance = this;
54.     }
55.    
56.     // Use this for initialization
57.     void Start()
58.     {
59.         ContainerObject = new GameObject("ObjectPool");
60.        
61.         //Loop through the object prefabs and make a new list for each one.
62.         //We do this because the pool can only support prefabs set to it in the editor,
63.         //so we can assume the lists of pooled objects are in the same order as object prefabs in the array
64.         Pool = new List<GameObject>[Entries.Length];
65.        
66.         for (int i = 0; i < Entries.Length; i++)
67.         {
68.             var objectPrefab = Entries[i];
69.            
70.             //create the repository
71.             Pool[i] = new List<GameObject>();
72.            
73.             //fill it
74.             for (int n = 0; n < objectPrefab.Count; n++)
75.             {
76.                
77.                 var newObj = Instantiate(objectPrefab.Prefab) as GameObject;
78.                
79.                 newObj.name = objectPrefab.Prefab.name;
80.                
81.                 PoolObject(newObj);
82.             }
83.         }
84.     }
85.    
86.     /// <summary>
87.     /// Gets a new object for the name type provided.  If no object type exists or if onlypooled is true and there is no objects of that type in the pool
88.     /// then null will be returned.
89.     /// </summary>
90.     /// <returns>
91.     /// The object for type.
92.     /// </returns>
93.     /// <param name='objectType'>
94.     /// Object type.
95.     /// </param>
96.     /// <param name='onlyPooled'>
97.     /// If true, it will only return an object if there is one currently pooled.
98.     /// </param>
99.     public GameObject GetObjectForType(string objectType, bool onlyPooled)
100.     {
101.        
102.         for (int i = 0; i < Entries.Length; i++)
103.         {
104.             var prefab = Entries[i].Prefab;
105.            
106.             if (prefab.name != objectType)
107.                 continue;
108.            
109.             if (Pool[i].Count > 0)
110.             {
111.                
112.                 GameObject pooledObject = Pool[i][0];
113.                
114.                 Pool[i].RemoveAt(0);
115.                
116.                 pooledObject.transform.parent = null;
117.                
118.                 pooledObject.SetActive(true);
119.                
120.                 return pooledObject;
121.             }
122.             if (!onlyPooled)
123.             {
124.                 GameObject newObj = Instantiate( Entries[ i ].Prefab ) as GameObject;
125.                 newObj.name = Entries[ i ].Prefab.name;
126.                 return newObj;
127.             }
128.         }
129.        
130.         //If we have gotten here either there was no object of the specified type or non were left in the pool with onlyPooled set to true
131.         return null;
132.     }
133.    
134.     /// <summary>
135.     /// Pools the object specified.  Will not be pooled if there is no prefab of that type.
136.     /// </summary>
137.     /// <param name='obj'>
138.     /// Object to be pooled.
139.     /// </param>
140.     public void PoolObject(GameObject obj)
141.     {
142.        
143.         for (int i = 0; i < Entries.Length; i++)
144.         {
145.             if (Entries[i].Prefab.name != obj.name)
146.                 continue;
147.            
148.             obj.SetActive(false);
149.            
150.             obj.transform.parent = ContainerObject.transform;
151.            
152.             Pool[i].Add(obj);
153.            
154.             return;
155.         }
156.     }
157. }

 

That line says it specifically.

Object.get_name is the method that generating the garbage.

Just so you know. Properties actually compile into 2 special functions, a 'get' function and a 'set' function. They're each named 'get_NameOfProperty' and 'set_NameOfProperty'.

So that's saying the 'name' property getter is generating garbage.

Specifically at this line:

line 145

Code (csharp):

1.  
2. if (Entries[i].Prefab.name != obj.name)
3.  

When you call the 'name' property (or the tag property as well) on unity objects, because it's calling to the internal unity engine, a brand new string is generated.

Strings aren't structs, instead they're an immutable reference objects (an array of chars to be specific).

This is actually why unity has a CompareTag method for tags:
http://docs.unity3d.com/ScriptReference/Component.CompareTag.html

Unfortunately there's no cheap comparename function.

What you could do is cache the name into the ObjectPoolEntry object when it gets generated, and you compare against that instead. So that way you don't generate new garbage.

Also... I notice you have an array of Lists called 'Pool'. But it appears the array really just matches index with the 'Entries' array. Meaning that the List at index 3 of Pool is matched to the ObjectPoolEntry at index 3 in Entries.

Why not instead make the List a member of ObjectPoolEntry, throw out the 'Pool' array all together, and now there's no chance of error where the arrays get out of sync. This can definitely happen because you've made those array publicly read/write, which means any code out there can modify those arrays. Remember to use your 'private' modifiers to denote objects that should be encapsulated into that class.


And also, in your 'DestroyOverTime' method, your update method is going to call invoke ResetEffect on EVERY update. This is also going to come with some massive overhead. That probably shouldn't be invoking so many times from the looks of 'ResetEffect'. Maybe do that in 'OnEnable' or 'Start'???



Something like this:

Code (csharp):

1.  
2. using UnityEngine;
3. using System.Collections;
4. using System.Collections.Generic;
5. public class ObjectPool : MonoBehaviour {
6.  
7.    #region Singleton Interface
8.  
9.    private static ObjectPool _instance;
10.  
11.    public ObjectPool Instance
12.    {
13.      get { return _instance; }
14.    }
15.  
16.    #endregion
17.  
18.    #region Fields
19.  
20.    [SerializeField()]
21.    private List<ObjectPoolEntry> _entries;
22.  
23.    private Transform _containerObject;
24.  
25.    #endregion
26.  
27.    #region CONSTRUCTOR
28.  
29.    void OnEnable()
30.    {
31.      if(_instance == null)
32.        _instance = this;
33.      else
34.      {
35.        Debug.Log("Duplicate ObjectPool instances were created, destroying extras.");
36.        Object.Destroy(this); //duplicates
37.      }
38.    }
39.  
40.    // Use this for initialization
41.    void Start()
42.    {
43.      //var go = new GameObject("ObjectPool");
44.      //_containerObject = go.transform;
45.      _containerObject = this.transform; //why not just use THIS gameobject?, also cache the transform to avoid implicit GetComponent calls
46.    
47.      //Loop through the object prefabs and populate the cached instances
48.      for(int i = 0; i < _entries.Count; i++)
49.      {
50.        var entry = _entries[i];
51.        entry.Name = entry.Prefab.name; //cache the name property for use later
52.      
53.        for(int n = 0; n < entry.Count; i++)
54.        {
55.          var newObj = Instantiate(entry.Prefab) as GameObject;
56.          newObj.name = objectPrefab.Prefab.name;
57.          //don't call PoolObject, it'll just do extra work
58.        
59.          newObj.SetActive(false);
60.          newObj.transform.parent = _containerObject;
61.          entry.Pool.Push(newObj);
62.        }
63.      }
64.    }
65.  
66.    #endregion
67.  
68.    #region Properties
69.  
70.    public List<ObjectPoolEntry> Entries { get { return _entries; } }
71.  
72.    #endregion
73.  
74.    #region Methods
75.  
76.    /// <summary>
77.    /// Gets a new object for the name type provided.  If no object type exists or if onlypooled is true and there is no objects of that type in the pool
78.    /// then null will be returned.
79.    /// </summary>
80.    /// <returns>
81.    /// The object for type.
82.    /// </returns>
83.    /// <param name='objectType'>
84.    /// Object type.
85.    /// </param>
86.    /// <param name='onlyPooled'>
87.    /// If true, it will only return an object if there is one currently pooled.
88.    /// </param>
89.    public GameObject GetObjectForType(string objectType, bool onlyPooled)
90.    {
91.      ObjectPoolEntry entry;
92.      for (int i = 0; i < _entries.Count; i++)
93.      {
94.        entry = _entries[i];
95.        if(entry.Name != objectType) continue;
96.      
97.        if(entry.Pool.Count > 0)
98.        {
99.          var go = entry.Pool.Pop();
100.          go.transform.parent = null;
101.          go.SetActive(true);
102.          return go;
103.        }
104.        else if(!onlyPooled)
105.        {
106.          var go = Instantiate(entry.Prefab) as GameObject;
107.          go.name = entry.Name;
108.          return go;
109.        }
110.      }
111.    
112.      //If we have gotten here either there was no object of the specified type or non were left in the pool with onlyPooled set to true
113.      return null;
114.    }
115.  
116.    /// <summary>
117.    /// Pools the object specified.  Will not be pooled if there is no prefab of that type.
118.    /// </summary>
119.    /// <param name='obj'>
120.    /// Object to be pooled.
121.    /// </param>
122.    public void PoolObject(GameObject obj)
123.    {
124.      var nm = obj.name;
125.      for(int i = 0; i < _entries.Count; i++)
126.      {
127.        if (_entries[i].Name != nm) continue;
128.      
129.        obj.SetActive(false);
130.        obj.transform.parent = _containerObject;
131.        _entries[i].Pool.Push(obj);
132.        return;
133.      }
134.    
135.    }
136.  
137.    #endregion
138.  
139.    #region Special Types
140.  
141.    /// <summary>
142.    /// Member class for a prefab entered into the object pool
143.    /// </summary>
144.    [System.Serializable]
145.    public class ObjectPoolEntry
146.    {
147.  
148.      /// <summary>
149.      /// the object to pre instantiate
150.      /// </summary>
151.      [SerializeField]
152.      public GameObject Prefab;
153.    
154.      /// <summary>
155.      /// quantity of object to pre-instantiate
156.      /// </summary>
157.      [SerializeField]
158.      public int Count;
159.    
160.      [System.NonSerialized()]
161.      public string Name;
162.    
163.      [System.NonSerialized()]
164.      public Stack<GameObject> Pool = new Stack<GameObject>();
165.    
166.    }
167.  
168.    #endregion
169.  
170. }
171.  

 

Thank you so much for helping us, but it ends up freezing our game during playmode.

When we put your script in there were a few errors. This is what we had to change to bypass the errors.



First, Change was that public class ObjectPool had to be changed to NewObjectPool. Because of the title of the script. Next I removed the underscore from the public static because in my spawner scripts that was referring my object pooler with instance not _instance. It made more sense to change it once in the object pooler vs all my spawner scripts:

Your code:

Code (CSharp):

1. public class ObjectPool : MonoBehaviour {
2.    #region Singleton Interface
3.    private static ObjectPool _instance;
4.    public ObjectPool Instance
5.    {
6.      get { return _instance; }
7.    }
8.  

Our code:

Code (CSharp):

1. public class NewObjectPool : MonoBehaviour {
2.    
3.     #region Singleton Interface
4.    
5.     public static NewObjectPool instance;
6.    
7.     public NewObjectPool _Instance
8.     {
9.         get { return instance; }
10.     }
11.    

Second I changed line 56....

your code:

Code (CSharp):

1.  newObj.name = objectPrefab.Prefab.name;

Our code:

Code (CSharp):

1. newObj.name = entry.Prefab.name;

After making all those changes and clearing the errors unity keeps crashing every time I try to start playmode.

 

We used on OnEnabled but the enemies now disappear on screen. Before when using update this wasn't happening.

what should we do?

 

Still in of assistance.

 

Not sure if it deals with the garbage collection but... You are invoking the reset effect with a 25 second timer once per frame (in update) as long as the object exists... you could technically have hundreds of those invokes line up waiting to go... which can't really be good. Maybe all of the references for those cached function calls are being released when it finally fires causing some garbage collection?

 

We changed that to OnEnabled just like LordOfDuct had suggested.

 

Yeah I got lazy apparently and didn't read all the way through. If your enemies disappear while still on screen... I'd either a) increase that invoke timer that recycles them to give them more time to do their thing or (I'm not really sure what your game is or how it works) b) ditch the timer and put a trigger off screen that "collects" and pools whatever enters it... is the garbage collection still an issue after you made @lordofduct's changes?

 

Good thing is that they are not disappearing on screen because each object in the pool's count is set to one.



However, our garbage collection actually seems worse. What I don't get is that only flipper 1 was sent back to the object pool but multiple objects are listed in the profiler, most of which are still in the pool (The inactive objects in the pool shouldn't be using ResetEffect ()).

Current objects in pool:



Active objects in scene:



Profiler:

 

First things first.

Encapsulation!

I made the actual field private and named '_instance' so that it was read-only from outside of the class. The public was a getter named 'Instance' just because that's my naming convention.

If everyone was already referencing it as 'instance'... don't change the field to be public and break encapsulation. Change the getter property to 'instance':

Code (csharp):

1.  
2. public class NewObjectPool : MonoBehaviour
3. {
4.  
5.    #region Singleton Interface
6.  
7.    private static NewObjectPool _instance;
8.  
9.    public NewObjectPool instance
10.    {
11.      get { return _instance; }
12.    }
13.  

Also:

And you have the error:


Here's the thing, I notice you changed the name 'objectPrefab' to 'entry', and the error is mentioning 'objectPrefab'.

I get a feeling you missed one of the references to 'objectPrefab' and didn't change the naming.




ALSO, note, when I posted my code I said:

When I say that, I'm not saying "This is your answer", I'm saying "here's the jist of what I mean".

I wrote all that code in the browser right here mid conversation... it was never compiled or even glimpsed at by an auto-complete or debugger. So I can't guarantee any of it would work.

I was merely showing you some guidelines of how to readjust your code.

You may come up with little errors like "this variable name doesn't match" (which you did)... these should be minor hiccups in adapting code you find on the internet. Do not expect copy pasted code from a web forum to 'just work'.




... reading on in the thread now.



So can we see the latest version of the code as it stands???

 

Ok Encapsulation is to new to me, so I didn't understand that what you were trying to do. So I'm wondering how do I access the getter with my spawner scripts?

Spawner Script:

Code (CSharp):

1. using System.Collections;
2.  
3. public class Spawner : MonoBehaviour {
4.    
5.         public GameObject Instance;                // The enemy prefab to be spawned.
6.         public float maxtime = 25;           // How long between each spawn.
7.         public float mintime = 10;
8.  
9.         //current time
10.         private float time;
11.        
12.         //The time to spawn the object
13.         private float spawnTime;
14.    
15.         public Transform[] spawnPoints;         // An array of the spawn points this enemy can spawn from.
16.  
17.         void Start ()
18.         {
19.             SetRandomTime();
20.             time = mintime;
21.  
22.         }
23.  
24.         void FixedUpdate(){
25.        
26.             //Counts up
27.             time += Time.deltaTime;
28.            
29.             //Check if its the right time to spawn the object
30.             if(time >= spawnTime){
31.                 Spawn();
32.                 SetRandomTime();
33.         }
34.        
35.     }
36.        
37.         void Spawn ()
38.         {
39.             time = 0;
40.  
41.             GameObject obj = NewObjectPool.instance.GetObjectForType("Orange Fish", true);
42.             if (obj == null) return;
43.  
44.             obj.transform.position = transform.position;
45.             obj.transform.rotation = transform.rotation;
46.             obj.SetActive (true);
47.  
48.         }
49.  
50.     //Sets the random time between minTime and maxTime
51.  
52.     void SetRandomTime(){
53.         spawnTime = Random.Range(mintime, maxtime);
54.     }
55. }

The line accessing The object pooler:

Code (CSharp):

1. GameObject obj = NewObjectPool.instance.GetObjectForType("Orange Fish", true);

Oh, I kind of thought that I just was sure what to rename.

I see, I'm sorry about that.

Of course, Object pooler script:

Note: this version is just like the original so I can test the game without throwing any errors. I have to go back and fix what you suggested the first time.

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. public class NewObjectPool : MonoBehaviour {
6.  
7.     /// <summary>
8.     /// Repository of commonly used prefabs.
9.     /// </summary>
10.     [AddComponentMenu("Gameplay/ObjectPool")]
11.     public static NewObjectPool instance { get; private set; }
12.    
13.     #region member
14.     /// <summary>
15.     /// Member class for a prefab entered into the object pool
16.     /// </summary>
17.     [System.Serializable]
18.     public class ObjectPoolEntry {
19.         /// <summary>
20.         /// the object to pre instantiate
21.         /// </summary>
22.         [SerializeField]
23.         public GameObject Prefab;
24.        
25.         /// <summary>
26.         /// quantity of object to pre-instantiate
27.         /// </summary>
28.         [SerializeField]
29.         public int Count;
30.     }
31.     #endregion
32.    
33.     /// <summary>
34.     /// The object prefabs which the pool can handle
35.     /// by The amount of objects of each type to buffer.
36.     /// </summary>
37.     public ObjectPoolEntry[] Entries;
38.    
39.     /// <summary>
40.     /// The pooled objects currently available.
41.     /// Indexed by the index of the objectPrefabs
42.     /// </summary>
43.     [HideInInspector]
44.     public List<GameObject>[] Pool;
45.    
46.     /// <summary>
47.     /// The container object that we will keep unused pooled objects so we dont clog up the editor with objects.
48.     /// </summary>
49.     protected GameObject ContainerObject;
50.    
51.     void OnEnable()
52.     {
53.         instance = this;
54.     }
55.    
56.     // Use this for initialization
57.     void Start()
58.     {
59.         ContainerObject = new GameObject("ObjectPool");
60.        
61.         //Loop through the object prefabs and make a new list for each one.
62.         //We do this because the pool can only support prefabs set to it in the editor,
63.         //so we can assume the lists of pooled objects are in the same order as object prefabs in the array
64.         Pool = new List<GameObject>[Entries.Length];
65.        
66.         for (int i = 0; i < Entries.Length; i++)
67.         {
68.             var objectPrefab = Entries[i];
69.            
70.             //create the repository
71.             Pool[i] = new List<GameObject>();
72.            
73.             //fill it
74.             for (int n = 0; n < objectPrefab.Count; n++)
75.             {
76.                
77.                 var newObj = Instantiate(objectPrefab.Prefab) as GameObject;
78.                
79.                 newObj.name = objectPrefab.Prefab.name;
80.                
81.                 PoolObject(newObj);
82.             }
83.         }
84.     }
85.    
86.     /// <summary>
87.     /// Gets a new object for the name type provided.  If no object type exists or if onlypooled is true and there is no objects of that type in the pool
88.     /// then null will be returned.
89.     /// </summary>
90.     /// <returns>
91.     /// The object for type.
92.     /// </returns>
93.     /// <param name='objectType'>
94.     /// Object type.
95.     /// </param>
96.     /// <param name='onlyPooled'>
97.     /// If true, it will only return an object if there is one currently pooled.
98.     /// </param>
99.     public GameObject GetObjectForType(string objectType, bool onlyPooled)
100.     {
101.        
102.         for (int i = 0; i < Entries.Length; i++)
103.         {
104.             var prefab = Entries[i].Prefab;
105.            
106.             if (prefab.name != objectType)
107.                 continue;
108.            
109.             if (Pool[i].Count > 0)
110.             {
111.                
112.                 GameObject pooledObject = Pool[i][0];
113.                
114.                 Pool[i].RemoveAt(0);
115.                
116.                 pooledObject.transform.parent = null;
117.                
118.                 pooledObject.SetActive(true);
119.                
120.                 return pooledObject;
121.             }
122.             if (!onlyPooled)
123.             {
124.                 GameObject newObj = Instantiate( Entries[ i ].Prefab ) as GameObject;
125.                 newObj.name = Entries[ i ].Prefab.name;
126.                 return newObj;
127.             }
128.         }
129.        
130.         //If we have gotten here either there was no object of the specified type or non were left in the pool with onlyPooled set to true
131.         return null;
132.     }
133.    
134.     /// <summary>
135.     /// Pools the object specified.  Will not be pooled if there is no prefab of that type.
136.     /// </summary>
137.     /// <param name='obj'>
138.     /// Object to be pooled.
139.     /// </param>
140.     public void PoolObject(GameObject obj)
141.     {
142.        
143.         for (int i = 0; i < Entries.Length; i++)
144.         {
145.             if (Entries[i].Prefab.name != obj.name)
146.                 continue;
147.            
148.             obj.SetActive(false);
149.            
150.             obj.transform.parent = ContainerObject.transform;
151.            
152.             Pool[i].Add(obj);
153.            
154.             return;
155.         }
156.     }
157. }

Destroy script:

Code (CSharp):

1. using UnityEngine;
2. using System.Collections;
3.  
4. public class DestoryOverTime : MonoBehaviour {
5.  
6.     public GameObject SpawnedObject;
7.     public float ReturnTime = 25;
8.     public bool poolAfterComplete = true;
9.    
10.     void OnEnable ()
11.     {
12.         if (SpawnedObject) {
13.        
14.             Invoke ("ResetEffect", ReturnTime);
15.         }
16.     }
17.  
18.     public virtual void ResetEffect ()
19.     {
20.         if(poolAfterComplete)
21.         {
22.             NewObjectPool.instance.PoolObject(SpawnedObject);
23.  
24.         } else {
25.  
26.             Destroy(SpawnedObject);
27.         }
28.     }
29. }
30.  

 

Still looking for an resolution...