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.
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;
}
publicstaticvoid 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), newobject[] { param, adapter, resultHandler });
}
privatestaticvoid ExecuteDbUpdateJob(object state) {
object[] array = state asobject[];
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);
});
}
}
publicdelegatevoid DbUpdateResultHandler(List<NPCLite> result);
void UpdateNpcsFromDb()
{
DbUpdateResultHandler jrh = new DbUpdateResultHandler(DbUpdateIsComplete);
DbUpdateAsync(0, jrh);
}
internalstatic 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.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
UpdateMaterial
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:
publicvoid 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.
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);
}
}
}
publicvoid 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.
cellsContainingNpcs is a dictionary of lists that we’re clearing and rebuilding on every DB update.
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.
//cellsContainingNpcs.Clear();
foreach (var key in cellsContainingNpcs.Keys) {
cellsContainingNpcs[key].Clear();
}
And that seems to improve things.
Pushed down to mere noise.
Dictionary accesses
Dictionaries can be expensive too.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.
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…
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
Media Molecule Dev Diary 1 - Way cool engine stuff starts 18 minutes in or so. (Of the people shown, I’ve met Alex and Mark and John Beech; all just as nice as you’d expect.)
Alex at Umbra Ignite 2015 - Fascinating stuff, and encouraging to see how R&D can go for even insanely talented folks.