Search Unity

How can I make this code faster?

Discussion in 'Scripting' started by tcz8, Jun 17, 2020.

  1. tcz8

    tcz8

    Joined:
    Aug 20, 2015
    Posts:
    504
    I want to get all unique materials from selected objects in the hiearchy (including childs).

    My code works but took forever to process 1500 GameObjects with no child.

    Is there anyway to speed this up?
    Code (CSharp):
    1. private void LoadUniqueMaterials() {
    2.     Renderer[] renderers;
    3.     List<Material> materials = new List<Material>();
    4.     foreach (GameObject g in Selection.gameObjects) {
    5.         renderers = GetComponentsInChildren<Renderer>(true);
    6.         foreach (Renderer r in renderers) {
    7.             foreach (Material m in r.sharedMaterials) {
    8.                 if (materials.Contains(m) == false) {
    9.                     materials.Add(m);
    10.                 }
    11.             }
    12.         }
    13.     }
    14.     objList = materials.Cast<Object>().ToList<Object>();
    15. }
     
  2. raarc

    raarc

    Joined:
    Jun 15, 2020
    Posts:
    535
    put a script in each game object that already has a reference to the renderer

    will cut quite some time since you wont have to waste time with getcomponent<renderer>
     
  3. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,537
    So for starters at line 5 in your code you say "GetComponentsInChildren" and not "g.GetComponentsInChildren". This right here is going to give you improper results since you keep calling GetComonentsInChildren on "this" and not on the gameobjects in your selection.

    Next... you can actually skip over your Seleciton.gameObject and apply a filter up front on the selection. Instead use the 'GetFiltered' method and pass in the desired type and filter style:
    https://docs.unity3d.com/ScriptReference/Selection.GetFiltered.html

    In this case you will probably want:
    Code (csharp):
    1. Selection.GetFiltered<Renderer>(SelectionMode.Deep)
    Here I filtered for Render deep through all the children. (it appears to handle disabled objects as well on deep)

    Next thing... alright, so you use a 'List' and you check if it 'Contains' the material to avoid duplicates in your result set. Here's the thing, 'Contains' has to iterate the entire list and check equality on every entry to see if it exists in the list. For small collections this is fast, but for large collections this can be slow. If all 1500 objects have at least 1 renderer with at least 1 unique material that means you're approaching a list of 1500 in length every loop that you call 'Contains'.

    There is an alternative collection out there called 'HashSet<T>':
    https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.hashset-1?view=netcore-3.1

    HashSet<T> doesn't allow duplicates by default. The collection is unordered, but it stores entries by calculating its hashcode and placing it in a bucket associated with that hashcode. This means that when calling 'Contains' it only has to check those objects that ended up in the same bucket (far fewer than the entire collection).

    I like using HashSet for "unique collections" as the capabilities are all built in. No need to even check Contains, if you add an entry to it that already is in it, it just doesn't do anything (Add actually returns true/false if the entry is successfully added or not).

    For small collections this actually is slower because the hash calculation and bucket search is more intensive than just looping a small number of objects. Since well... a bucket is a small number of objects AND you have to calculate the hash. It's obviously more work. BUT, as the collection grows, it becomes faster. Because that hashcode and looping 10 or so objects is faster than looping 1500 objects.

    So with that said... here is a solution:
    Code (csharp):
    1.  
    2.         var hset = new HashSet<Material>();
    3.         foreach (var r in Selection.GetFiltered<Renderer>(SelectionMode.Deep))
    4.         {
    5.             foreach (var m in r.sharedMaterials)
    6.             {
    7.                 hset.Add(m);
    8.             }
    9.         }
    10.         objList = hset.Cast<UnityEngine.Object>().ToList();
    11.  
    For small numbers of objects, your solution is actually faster. But we're talking nanoseconds here. You're not going to notice much.

    But when you're talking a large number of unique materials. This hashset is WAY faster. I did it on 1500 Renderer's with a unique material on each and your solution took 100ms, my solution took 2ms. (your numbers will likely vary from mine depending on the CPU you have... the point is that the hashset is faster by a 50x scale)

    ...

    Another thing.

    Is there a reason you Cast to 'Object' in the end? If you need it as an Object list, just type it as such from the get go. This avoids having to run the 'Cast'. This isn't that much of a performance boost... but it's still a slight performance boost:
    Code (csharp):
    1.  
    2.         var hset = new HashSet<UnityEngine.Object>();
    3.         foreach (var r in Selection.GetFiltered<Renderer>(SelectionMode.Deep))
    4.         {
    5.             foreach (var m in r.sharedMaterials)
    6.             {
    7.                 hset.Add(m);
    8.             }
    9.         }
    10.         objList = hset.ToList();
    11.  
    And finally this is all going to be garbage intensive. There's actually methods that allow taking in lists to get these results which avoids the array allocations that are going on. Thing is it actually slows things down, but avoids the garbage. Considering that this appears to be an editor script... it's not necessary. Who cares if the editor spits out some garbage.

    [edit]
    Oh and to add onto this... HashSet actually has various set methods like Union and Intersect. You can use the Union method to behave similar to an 'AddRange' like so:

    Code (csharp):
    1.  
    2.         var hset = new HashSet<UnityEngine.Object>();
    3.         foreach (var r in Selection.GetFiltered<Renderer>(SelectionMode.Deep))
    4.         {
    5.             hset.UnionWith(r.sharedMaterials);
    6.         }
    7.         objList = hset.ToList();
    8.  
    It's technically ever so slightly slower than the foreach, and it's just because of some safety calls in the 'UnionWith' method (it checks null and calls a contract method). But when I say slightly slower... I'm talking like 1/10th of a millisecond over 1500 calls.

    Anyways, it shortens the code up even more with little added overhead.

    I did try a version using all linq and it doubled the time over this solution, so I avoided that.
     
    Last edited: Jun 18, 2020
    Kurt-Dekker likes this.
  4. tcz8

    tcz8

    Joined:
    Aug 20, 2015
    Posts:
    504
    You know when you find a stupid mistake in code you just posted, rush back to update it, but someone already pointed it out...? :oops::D But wow! No wonder it took forever... it added up to over 2,250,000 objects being processed!!!

    Thanks a bunch for all the recommendations I will apply this right away!
     
  5. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,537
    I definitely know that feel.