I think the cache is a good idea, especially if there are many los checks being performed without the mobs moving, and as you've seen in your tests it does seem to be a performance win.
The only concern I have is that I don't see anything ever being removed from the cache. It'll get cleaned up in the destructor when a mob dies, but any other mob with entries pointing to the now destroyed mob won't go away.
Do mobs regularly perform los checks on each other? I suppose the worry is that in a zone where many mobs are killed over and over but there are some that are never killed, then those caches would continue to grow over time as the entries that point to the dead mobs accumulate.
The only way I can see to keep that from happening would be to tell every other mob to remove any entries pointing to a mob that is being destroyed from their cache. That would mean a potentially costly operation whenever a mob dies. An alternative could be to have a scheduled task on each that dumped entries that were very old, but then that will cause a periodic cpu spike if they all do it at the same time.
Have you seen memory usage creeping up over time if you kill some mobs but not all of them? It might be worth logging out the size of each cache periodically for debugging purposes and see if they just grow forever on any mobs with long lifetimes.
|