ElgarL/Towny — pull request #39 by Zren (2012-07-20 08:21:21+00:00)
Title: MobRemoval Refactor

Warning, I didn't test this yet. It needs another pair of eyes to make sure the code's logic is the same (I'll relook at it later with a fresh head).

Zonedabone came to complain about MobRemoval being slow in IRC. While he's use case is pretty bad. (I glossed over the fact his db is networked when I was talking too him), he got me to look at the code, and realized it could be better. So I refactored it a little.

I removed the old commented out code (git will keep it in history). I also changed all the negative conditions that got nested up to if (condition) continue. I wrote a function to abstract away the copy pasted code for world + town mob classes. I also removed the wacko string comparison...

The only time this part would fire is if...
You were removing, Spider, but not WolfSpider or something. But WolfSpider doesn't inherit Spider/
Ew. Delete!

His use case isn't really the norm. Network latency wouldn't spike his CPU. He's use case doesn't work (I only just though of it now) as we remove mobs in the unclaimed areas as well.

The only way to improve on this would be for massive scaled dbs where map lookups would be incredibly slow. And we'd have to break all of the towny checks out of the main thread. That creates a whole buttload of threading fun. If that were the case, it'd be better to move this to a LivingEntityMoveEvent (which doesn't exist). And filter it down to a LivingEntityMoveBlockEvent.

IRC Logs:

It looks like mob removal is taking quite a bit of main thread time.
I suggest that it be changed slightly.
First, in a separate thread, get a list of all chunks that are towns.
Next, in the main thread, iterate over those chnks and delete the right mobs inside them.
THis wwould greatly improves speed on our server, as there are many, many chunks with many, many mobs, but very few town chunks
At leat thread data source functions
...
Towny mob removal on <3k mobs brought our server to 100% cpu on an i7
Getting the entities *should* be in the main thread.
...

TownyWorld townyWorld = null;
try {
townyWorld = TownyUniverse.getDataSource().getWorld(world.getName());
} catch (NotRegisteredException e) {
// TODO Auto-generated catch block
e.printStackTrace();
return;
}

Because right now this lookup is being done ~3k times on my server when it could be done 20 times.
I don't know how expensive that lookup is, but chances are since we use a network database it's very expensive.
Especially for the main thread.