search

Terasology / DynamicCities

Cities that get fancy!
17 stars 13 forks source link

Troubleshoot performance issues, improve monitoring #34

Open Cervator opened 6 years ago

Cervator commented 6 years ago

Starting recently I've noticed an occasional massive performance drop with DC active, both via MedievalCities and TutorialDynamicCities. Unsure if it came in during GSOC 2018 with some related ST changes or just has been hiding for a while.

It is hard to troubleshoot as I'm not sure yet how to reliably trigger it. Sometimes it'll happen before I even find the first village, other times I'll find one village and everything will be fine, then go off hunting another and suddenly the game will slow to a crawl. I fly around with ghost + hspeed

I suspect it might have something to do with scanning the world for suitable settlement sites, but I have no evidence for that. It just seems to trigger based on a number of chunks generated or from characteristics of specific chunks having generated.

To better troubleshoot, even if not resolving the issue, the goal here is to review likely spots for performance problem possibilities and better log/monitor to make finding them easier. When this slowdown occurs there is no unique monitoring category that spikes, when there really should be.

For an example see StateInGame which wraps several monitoring activities around various possible points that might take a while or become complicated in the future. Find similar spots in DC and add a unique new activity.

Example snippet:

        PerformanceMonitor.startActivity("Rendering NUI");
        nuiManager.render();
        PerformanceMonitor.endActivity();

Example target: The update method in SettlementEntityManager, specifically the two for loops. It may also make sense to move the first if block to within the throttling counter:

        if (counter != 0) {
            return;
        }
...
        counter = 250;

(The stuff represented here by ... is what I think we should try wrapping a monitor activity around)

I'm not sure if the stuff in the preceding if block really needs to happen before that counter check. Moving it past it would save that block from running 249 times out of 250 ticks.

Cervator commented 6 years ago

It may be worth reviewing #31 in light of these issues, and see if the new API would be cleaner and perhaps more performant, then wrap monitors around the new stuff instead.