ssilk wrote: Fri May 13, 2022 5:36 am
This is a typical case, where adding an index would make things faster. In special cases.
A) don’t you think the last slot of a chest isn’t already cached? Why?
B) general usage of index would lead to a very big index compared to the slots if there are many different things in a chest. So the algorithm needs to lookup the right index first and then lookup the slot. Instead of looking for the first slot with this item.
C) it’s always a bad idea (not only for Factorio) to take a special case and improve around that.
D) Can you prove, that your algorithm is always — especially in the most used cases — faster than yet.
In other words: I don’t think it makes any sense, this is already highly optimized. But you have luck, because I have too less implementation knowledge to move it to won’t implement.
First, my apologies, I don't think I explained it very well.
For A, I don't know for sure that some sort of indexing isn't already in play. However the fact that insertion/removal logic behaves exactly as if it's iterating over every slot, and very large containers (warehouse mods, etc) are notorious for causing lag unless used sparingly, suggests it.
There have been threads discussing this and talking about adding indexing of some sort before, though I decided to make a new thread because i'm suggesting a specific indexing method.
For B and C and D, i'm not suggesting a general index for every item, i'm suggesting a minimalist index that should improve performance significantly in some usecases but
barely affect performance in others. Assuming I'm understanding how inserters and such function, anyway. Let me provide some illustrations to better explain:
We have a chest.
When a inserter, robot, etc tries to add items to a chest, it iterates over every slot starting from the first slot.
For every slot it iterates over, it checks if there is a stack it can add the item onto. Meaning, if it's the same item type, if the stack isn't full, etc. If there is such a thing, it adds to the first one it finds.
And if it doesn't find any partial stacks to add to after iterating over the whole length of the chest...
...It goes back and adds a stack at the first free slot it found.
Removing items is much the same process, except the iteration starts from the last slot and goes backwards, and additionally checks if each slot meets any filter criteria (filter inserter, inserting into a assembler, etc).
Now, here's the interesting bit - if you're just adding or removing the same items from a chest, and you've already found the slot to do so with, there's no reason to iterate over the slots again - when adding items, the stacks can only be added
in the direction that it iterates in, and likewise for removing items. So you can just remember where you last added or removed a item, and start from there.
If a stack is filled, it still puts the next stack the correct slot.
And in the worst case, if a stack is removed, it just has to iterate over all the contents to find the right spot again.
Which is the
same worst case scenario as right now, when a chest is almost entirely full and it has to add to the stack on the end.
Now of course, this implementation would deviate from current behavior if we removed a stack in the middle somehow.
But conveniently, since inserters add items from the start onwards, and remove items from the end, inserters
can't do that.
(note, removing items would have to have a check added for partial stacks just like adding items, in order to get consistent behavior for both halves of this coin. This shouldn't be a big deal.)
Now it's easy to see on it's own how this would be a great optimization - the worst case for this optimization (iterating the entire chest) here happens whenever a stack is filled or emptied in the chest, while the worst case right now happens
every time a item is added to a almost full chest or removed from a almost empty chest. Since stacks take many item addition/removal operations to fill or empty,and this only requires adding a integer or two and a slight change to the iteration algorithm (to check for and start from a different index number than "0"), this is optimization is certainly better... in this limited scenario.
But, you're right. There's a scenario where it breaks down. Two scenarios, actually:
1: Things that don't play by the rules
2: Multiple item types
For #1, things that don't play by the 'rules' means that it is something that doesn't iterate from start on insertion and iterate from the end on removal. A example would be the player; a player can trivially remove a stack from the
middle of a chest...
...and thus break this nice optimization instantly.
Thankfully, this is fairly easy to resolve: any time a player, or other 'rule breakers', touch a chest at all, just reset the 'last modified slot' index so that inserters fall back to default behavior. Since players will typically interact withchests around the factory
much less frequently than inserters, since inserters outnumber players a thousand-to-one, this isn't a big deal. Also, it just means that the worst case is, again, about the same as the worst case right now.
And finally, the elephant in the room, #2. When chests have only one item type going in or out, this thing works fine. But slots are shared between all itemtypes, and these indexes are (as described) agnostic of item type. So if you have more than one item in a chest...
...and a filter inserter, or something that causes a inserter to filter (inserting into assembler, filtered container, etc), removes a item or stack in the
middle...
...then there are a variety of ways for it to break.
Now, there are some ways you could try to solve that, each with upsides and downsides.
- You could make any 'filtered' interaction with a chest at all reset the indexes just like player interaction, but that denies the optimization in almost all of the situations it would be used - all bot interactions, any time a inserter is taking from a chest and putting in to a assembler, etc.
- You could store the type of item along with the 'last interacted' index, or store one index per item in the chest, etc. But these add complexity and additional memory reads and writes and operations to the process, all of which will diminish the efficacy of the optimization and increase the performance losses outside of the areas where it improves performance.
- You could disable the optimization on any chest with more than one item type. A bit of a mixed bag, this would certainly help in many situations (e.g. a warehouse mod storing 1,000 stacks of iron ore), but not in others (requester chests with multiple items).
And more.
Someone at Wube, with actual access to the code, could probably do a better job of figuring out how they'd patch the holes in the optimization. But I think there's enough merit here to warrant investigating it, especially since it's really not that big of a change. Depending on the details, this change might only need to touch a couple dozen lines of code. Testing it could take longer than changing it. Compared to the massive efforts they've gone through to optimize bots and belts and so on, that's nothing.