Skip to main content
  1. Posts/

Day 25 - Performance Enhancement 2

OldDays ste-reez-muvi Unity csharp

Let’s take the sting out of NPC updates as well.


Cool Threads for NPCs
#

Not those kinds of threads.
Not those kinds of threads.

Experimenting with moving the map updates to a separate thread was successful, so let’s try with the NPCs. Luckily we were already calling a static method to hit the DB, and we can use a formula very similar to what we did in MapManager.

Assets/Managers/NpcManager.cs
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
void Update () {
  if (dbUpdateComplete && Time.time > nextDBUpdate) {
    print("Time to read from the DB! " + Time.time);
    nextDBUpdate = Time.time + dbUpdateDelta;
    dbUpdateComplete = false;
    activeGameRounds.Clear();
    cellsContainingNpcs.Clear();
    UpdateAllNpcs();
    UpdateNpcCohab();

    UpdateNpcsFromDb();
  } 
}

void DbUpdateIsComplete(List<NPCLite> npcs)
{
  print("DB update complete! " + npcs.Count + " NPCs updated!");
  npcsFromDb = npcs;
  dbUpdateComplete = true;
}

public static void DbUpdateAsync(int param, DbUpdateResultHandler resultHandler = null)
{
  //If callback is null; we do not need unity adapter, otherwise we need to create it in ui thread.
  ThreadAdapter adapter = resultHandler == null ? null : CreateUnityAdapter();

  System.Threading.ThreadPool.QueueUserWorkItem(
    //jobResultHandler is a function reference; which will be called by UIThread with result data
    new System.Threading.WaitCallback(ExecuteDbUpdateJob), new object[] { param, adapter, resultHandler });
}

private static void ExecuteDbUpdateJob(object state) {
  object[] array = state as object[];

  int param = (int) array[0];
  ThreadAdapter adapter = array[1] as ThreadAdapter;
  DbUpdateResultHandler callback = array[2] as DbUpdateResultHandler;

  //... time consuming job is performed here...
  List<NPCLite> result = DragonsSpine.DAL.DBNPC.GetAllNpcs();

  // if adapter is not null; callback is also not null.
  if (adapter != null) {
    adapter.ExecuteOnUi(delegate {
      callback(result);
    });
  }
}

public delegate void DbUpdateResultHandler(List<NPCLite> result);

void UpdateNpcsFromDb()
{
  DbUpdateResultHandler jrh = new DbUpdateResultHandler(DbUpdateIsComplete);
  DbUpdateAsync(0, jrh);
}

internal static ThreadAdapter CreateUnityAdapter()
{
  GameObject gameObject = new GameObject();
  return gameObject.AddComponent<ThreadAdapter>();
}

void UpdateAllNpcs() {
  List<NPCLite> npcs = npcsFromDb;

A look at the profiler is a bit disappointing, though.

Still too spikey on the graph.
Still too spikey on the graph.
Our code is expensive.
Our code is expensive.

It seems we’re still doing expensive things in our NPC updates; outsourcing the DB delay wasn’t that impactful.


Tracking down the culprits
#

No disintegrations.
No disintegrations.

UpdateMaterial
#

Constantly hitting the Renderer like it owes us money.
Constantly hitting the Renderer like it owes us money.

It looks like we make a lot of calls to NpcScript.UpdateMaterial that are kind of expensive. Not surprising:

Assets/NpcScript.cs
121
122
123
124
125
126
127
128
129
public void UpdateMaterial(int currRound) {
  if (lastActiveRound >= currRound-1 && Health > 0f) // one round leeway in case we catch the DB in mid-update
    myRend.material = liveMaterial;
  else {
    presumedDead = true;
    myRend.material = deadMaterial;
  }
  myRend.material.mainTextureScale = new Vector2(-1f,-1f);
}

We set the material and its scale every single time, whether anything’s changed or not. That we can fix, keeping track of the material and hitting the actual Renderer only when it changes.

Assets/NpcScript.cs
27
28
29
30
31
32
33
34
35
36
37
private Material currMaterial;
public Material CurrMaterial {
  get { return currMaterial; }
  set {
    if (currMaterial != value && myRend != null) {
      currMaterial = value;
      myRend.material = currMaterial;
      myRend.material.mainTextureScale = new Vector2(-1f,-1f);
    }
  }
}
Assets/NpcScript.cs
133
134
135
136
137
138
139
140
public void UpdateMaterial(int currRound) {
  if (lastActiveRound >= currRound-1 && Health > 0f) // one round leeway in case we catch the DB in mid-update
    CurrMaterial = liveMaterial;
  else {
    presumedDead = true;
    CurrMaterial = deadMaterial;
  }
}

List growing
#

We spend a lot of time growing a list.
We spend a lot of time growing a list.

cellsContainingNpcs is a dictionary of lists that we’re clearing and rebuilding on every DB update.

Managers/NpcManager.cs in Update
62
63
cellsContainingNpcs.Clear();
UpdateAllNpcs();

It seems to me that cells which contain NPCs this round are likely to contain NPCs in future rounds. We can clear those individual lists without removing them entirely, so they don’t have to re-grow next time. I understand that making a new List can sometimes be faster, but I don’t know ahead of time how large the list will be, and want to keep the garbage collector as idle as I can manage.

Assets/Managers/NpcManager.cs
63
64
65
66
//cellsContainingNpcs.Clear();
foreach (var key in cellsContainingNpcs.Keys) {
  cellsContainingNpcs[key].Clear();
}

And that seems to improve things.

Pushed down to mere noise.
Pushed down to mere noise.

Dictionary accesses
#

Dictionaries can be expensive too.
Dictionaries can be expensive too.
In both update functions.
In both update functions.

Let’s spend a little time optimizing our use of the various dictionaries. Two important details about C# dictionaries: First, TryGetValue can be a lot more efficient than checking if a key exists and then grabbing the value if it does. Second, KeyValuePair is a lot better than iterating through the list of keys if you’re going to need the values anyway.

Assets/Managers/NpcManager.cs
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
void UpdateAllNpcs() {
  List<NPCLite> npcs = npcsFromDb;
  // print("I found " + npcs.Count + " NPCs");
  numNpcsInDb = npcs.Count;
  foreach (NPCLite npc in npcs) {
    if (npc.lastActiveRound > maxGameRound) maxGameRound = npc.lastActiveRound;
  }
  foreach (NPCLite npc in npcs) {
    // skip inactive NPCs, and make sure they know they are inactive
    if (npc.lastActiveRound < maxGameRound - 10) {
      NpcScript tempNpcScript;
      if (npcScripts.TryGetValue(npc.worldNpcID, out tempNpcScript)) {
        tempNpcScript.gameObject.SetActive(false);
      }
      npcLocations.Remove(npc.worldNpcID);
    } else {
      activeGameRounds.Add(npc.lastActiveRound);
      CellLoc cell = new CellLoc(npc.X, npc.Y, npc.Z);
      npcLocations[npc.worldNpcID] = cell;
      if (!cellsContainingNpcs.ContainsKey(cell)) 
        cellsContainingNpcs[cell] = new List<int>();
      cellsContainingNpcs[cell].Add(npc.worldNpcID);
      Vector3 position = CLUtils.CellLocToVector3(cell, 1);

      NpcScript tempNpc = null;
      if (npcScripts.ContainsKey(npc.worldNpcID)) {
        tempNpc = npcScripts[npc.worldNpcID];
      } else {
        // Try to find an inactive one to re-use
        tempNpc = null;
        int tempNpcId = 0;
        foreach (KeyValuePair<int,NpcScript> kvp in npcScripts) {
          if (!kvp.Value.gameObject.activeSelf) {
            tempNpc = kvp.Value;
            tempNpcId = kvp.Key;
            break;
          }
        }
        if (tempNpc != null) {
          print ("Found inactive NPC " + tempNpcId + " and am reusing it!");
          npcScripts.Remove(tempNpcId);
          tempNpc.Reset();
        } else {
          print ("Could not find an inactive NPC for " + npc.worldNpcID + " so am instantiating it!");
          tempNpc = (NpcScript) Instantiate(npcScript);
          tempNpc.npcManager = this;
        }
        tempNpc.npcId = npc.worldNpcID;
        tempNpc.name = npc.Name;
        SetMaterials(tempNpc);
        tempNpc.toBeSeen = (position.z <= mapManager.zTop + 1);
        tempNpc.gameObject.SetActive(true);
        npcScripts[npc.worldNpcID] = tempNpc;
      }
      tempNpc.cell = cell;
      tempNpc.newPosition = position;
      tempNpc.lastActiveRound = npc.lastActiveRound;
      tempNpc.UpdateMaterial(maxGameRound);
      tempNpc.Hits = npc.Hits;
      tempNpc.HitsFull = npc.HitsFull;
      tempNpc.MostHatedId = npc.mostHatedId;
      tempNpc.HasMostHated = npc.hasMostHated;
    }
  }
}

That worked pretty well; the spikes were still there, though not as severe.

One more thing…
#

Well, there goes another evening&hellip;
Well, there goes another evening…

I tried moving the NPC update methods onto the separate thread with the DB update; the resulting code is in a branch called TheMess which will not be pulled into master. In retrospect it seems obvious, but nothing that touches a GameObject in any way can be on a separate thread. This includes instantiations, position/scale changes, material shifts, even reading any of those values or loading assets. If performance becomes an issue again, I’ll need to seriously consider how to draw the border between on- and off-thread code.


A dreamy aside
#


More to come
More to come

Day 25 code - visualizer