IslandzVW / halcyon

InWorldz Halcyon 3d virtual reality world simulator
BSD 3-Clause "New" or "Revised" License
21 stars 26 forks source link

Remove duplicate key check in SceneView object culling path. #389

Closed kiranoot closed 7 years ago

kiranoot commented 7 years ago

This removes a call to m_updateTimes.ContainsKey around a call to m_updateTimes.Remove. The Dictionary.Remove call throws no exception on a missing key, and there are no side effects on a missing key. This very slightly optimizes the path by avoiding a duplicate key exists check inside the dictionary.

As an aside, I am not familiar enough with the code to know if part.ParentGroup.GetParts() will always contain part.LocalId or not. If there is not a guarantee that part.LocalId is present within the returned parts, it needs to be removed manually.

appurist commented 7 years ago

I've confirmed this through testing in the code too (I don't always trust the MS docs to reflect accuracy for all older .NET releases as well as current), but in this case you can call .Remove whether the key is found or not. So this is a pattern we can use in general going forward (or rather, the pre-check is an anti-pattern to avoid). There are probably numerous places we can update, and I'll start including that in future commits as I touch a bit of code. This one is ready for merge.

(P.S. Good catch, I was completely unaware that it wouldn't throw an exception, given an unknown key!)