StorageDrawers: Performance problems with massive item transfers

A heads up as I am currently testing some massive/extreme cases of item transfers with AE2 in 1.10 for IItemHandler. Using 1.10.2-3.2.7 for it.

I am speaking about item transfers it the range of 10k-100k items/t and a cube with 12x12x(4 to 12) drawers, which certainly fits the ridiculuous level. But that is where the magic starts 😉.

Attached a visualvm snapshot. Most notable is the Vec3i.hashCode(), which is actuall used by TileEntityController.isDrawerEnabled / TileEntityController.getGroupForCoord. Thus most is not actually spend in inserting or extracting the items but finding the drawers and if they are enabled.

https://dl.dropboxusercontent.com/u/7021308/1.10.2-storage-drawers.nps

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 17 (9 by maintainers)

Most upvoted comments

Does this release offer any meaningful improvement?

https://github.com/jaquadro/StorageDrawers/releases/tag/sd-3.4.0-a1

I’ve gone after some of the lower-hanging fruit. Avoiding world lookups and coordinate hashing for most of the controller ops, and fusing the enable/get operation everywhere else.

Looks like a pretty good improvement.

The Vec3.hashCode() part is completely gone.

It is still nothing you really want on your server. The case with insert 6 * 2k/t (different items) is somewhat acceptable. For me it takes around 1/3 of the server thread. Probably ok for a SP. But then seriously. If I did not overlook an upgrade, each drawer has a max capacity of 2080 stacks and a 12x12x12 cube would be filled in 15min. Placing down about 166k drawers each day is ridiculuous.

What now appears is that ItemStack.copy() sucks. Hard. To some degree it is AE2s fault, as we do a simualted extract for every inject to prefer inventories, which already contain these items. It might be an idea to move from an extract to query a cache. But that might be problematic as it would now also consider read only inventories as potential destination. But that might be an acceptable tradeoff. In a few cases I saw getStackInSlot turn up in the sampling as it copies the itemstack. The docs state nobody should modify them directly. But I can understand taking precaution as there are certainly some devs who will ignore it.

Out of curiosity, if have change the order we iterate over the inventory, so we enforce using the virtual slot in the last positon. Which basically changed the not that insane test with 12k/t from taking about 1/3 third of the CPU time for the server to just high enough that the sampler can pick it up. So having the first one being virtual would be a major improvement for inserting. (Real) Extracting is still bad thanks to ItemStack.copy(), but that is not something we can fix. Also keeping both the last and first slot as virtual ones might be a good idea. I was thinking about reversing the order for extract. Thus it prefers a stack on the end of the inventory and it is more likely to keep the start of an inventory untouched, should someone sort it by hand.