Storage Drawers: Forestry Pack

Storage Drawers: Forestry Pack

6M Downloads

Expensive item copy showing up in profiler

Ricket opened this issue ยท 4 comments

commented

ItemStack stack = drawer.getStoredItemPrototype().copy();

This copy line shows up in my profiler as taking at least 8% of my tick time. I have several storage drawers clusters attached to AE2 storage buses, and also EnderIO conduits using storage drawers, they're all showing hot in the profiler because they're calling getStackInSlot (presumably to search for items) and the copy is expensive. Is there a way to optimize this code to eliminate the copy?

I tried to add a WeakHashMap<ItemStack, ItemStack> which would have the key of the prototype and the value of the copy, and then just return the value, but it caused some bugs. Bugs still happened if I verified the value's count matched, and also if I verified ItemStackMatcher.areItemsEqual, so I couldn't quite get it to work. I feel like an experienced modder could figure it out, though.

commented

I'll have to think hard about why I can't just sync the count with the prototype and always return the prototype directly. IItemHandler's definition has strong language that the returned value should never be modified. It might have just been for added safety.

commented

Can you store the first copy with the IDrawerGroup so that the prototype is only ever copied once per slot? (then this method would just update that object and return it) -- Or is it assumed that the object returned by getStackInSlot can be modified by the caller?

(I wish ItemStack was immutable, then this would be so much easier to reason about)

commented

The stack returned should never ever be modified (this is what the interface javadoc is literally screaming). Does it still happen in the wild? I'm not sure. Minecraft set a bad example with standard containers and stacks modified with complete abandon.

So it might be safe. Consumers still need to be careful that they make their own copy or re-fetch the stack if they modify any~~ slot in the inventory, and MAYBE this is where someone could get into trouble.

commented

This has been addressed in the 5.4.0 release, by maintaining a separate "public stack" alongside the prototype stack, and setting its count and returning it directly in the get calls. This should be safe if other mods obey the scary allcaps warnings, and it will eliminate the get copy.

Poke around with it if you get a chance.