Blu3wolf / Treefarm-Lite

This mod enables you to cultivate trees or other plants
GNU General Public License v3.0
11 stars 9 forks source link

Performance hit when field output inventory is full #31

Closed Blu3wolf closed 8 years ago

Blu3wolf commented 8 years ago

Reports of performance hits in 0.4.0 when output inventory is full - #30 @secondadvent and @StephenWard

Blu3wolf commented 8 years ago

Another issue I've noticed is that they cause massive slowdown (from 60 fps/ups to 15-20 of both with my 30 farms) if they get their output of wood backed up. I recall loading up my save one day and getting massive stuttering and slowdown, which only went away after running to my fields and manually removing the wood - I didn't have a buffer because I wanted the trees to stay grown until needed to help reduce pollution.

After putting in a large buffer I haven't had any trouble, except in the uncommon times where a bunch of trees mature at once and my output can't keep up, then it's just a second or two of stutter. Other than this I haven't had any performance issues with the treefarms going.

Blu3wolf commented 8 years ago

Yeah, I just updated from the pull request version to the actual 4.0 release build and it's still an issue. With my 30 farms intentionally backed up (just to speed up the process) I'm currently dropping from my normal 60 to around 40 fps/ups, though the fields just harvested a bunch of the trees so there's not as many mature ones as the last time they backed up. As more trees mature my speed continues to drop, and as soon as I clear all of the wood from the inventories, even with the belts still backed up, the speed returns to normal.

Blu3wolf commented 8 years ago

Im looking at line 542 presently. Doesnt look like it should be problematic...

Blu3wolf commented 8 years ago

Im also not understanding the comment on line 880.

StephenWard commented 8 years ago

@Blu3wolf line #880 along w/ the mk1_harvest_tree() function are the problem. In mk1_harvest_tree, get_output_inventory(), game.item_prototypes[], and get_item_count() will all be slow, although game.item_prototypes[] probably beats the others by a bit.

Thankfully, performance is no worse than it was in v3.7 for these farms.

To understand the performance issue, with 30 farms, there will be a farm updating every other tick. Each farms supports about 60 trees. So every other tick, in the call on line #880, the mod checks 60 trees to see if it can harvest them - when they are immature they fail on line #596 and no further work is done.

However, when the tree is ready to harvest, mk1_harvest_tree() has to get the inventory, check the item prototype for the stack size, ask the inventory if it can insert the items, and then insert the harvest into the inventory. All of those actions are comparatively expensive b/c they are C calls that don't do a lot of work. So when all 60 trees are mature, and the inventory is full, then that work is done 60 times every other tick - uselessly.

Ideally, if inventory was full, the mod wouldn't even try to harvest. There are some things that need to be kept in mind for any potential fix though:

[Edit] As well, there are the following complications w/ the Factorio lua api

Blu3wolf commented 8 years ago

That would seem problematic. Ill not have time to look closely at it for a couple weeks, unfortunately.

Matrix0150 commented 8 years ago

I sent you a very simple fix to stop the check for all same named entity if one fail. All the field will still check at least once for each type so there's still a small hit when it's full, but it should be up to 60 time better on a full mature field.

Further optimization could be done as the game will still lag if you have a very large quantity of full field but it should be good enough most of the time.

Blu3wolf commented 8 years ago

Published 0.4.1 with @Matrix0150 s performance fix.

secondadvent commented 8 years ago

With this change my 30 farms backed up and filled with mature trees is only showing a fairly minor hit now - sticking around 57+ ups even zoomed out (with my other stuff going as well) where before the game would be struggling even fully zoomed in. Definitely an improvement :3

Blu3wolf commented 8 years ago

The other fix of course being to switch to the drone towers, which dont have an output inventory to worry about!

Technically on 0.4.1.1 at this point, tags are not quite in the right spot.

Zanthras commented 8 years ago

Sadly the drone towers have one glaring weakness.... they never stop harvesting. At least I couldnt find a way to make them stop. (short of stopping seedling production to prevent new plantings which would then stop harvesting)

Blu3wolf commented 8 years ago

Further optimization probably still possible, but the glaring issue is fixed. Cheers!