Advanced Crafting Table excess tick times in certain circumstances
RoyCurtis opened this issue ยท 5 comments
It appears that the Advanced Crafting Table is performing an expensive iteration of recipies per tick, if:
- It contains no items in its crafting grid
- It contains items in its crafting grid that do not fit any recipies
In a modpack such as Direwolf20 with many recipies, this results in up to 3 milliseconds of tick time taken; an excessive amount for a single tile entity. This adds up per loaded ACT.
Details
Discovered on
- Direwolf20 1.7.10 1.4.1, 1.5.0 and 1.6.1 live server
- Forge 10.13.4.1448
- Buildcraft 7.0.13, 7.0.17 & compat 7.0.8, 7.0.9
Screenshots
- Opis graph of tick time whilst adding many ACTs
- Opis timings report with many empty ACTs
- Opis timings report with ACTs that have items in them
- Java Flight Recording of hot methods with many empty ACTs
- Java Flight Recording of hot methods with ACTs that have items in them
Reproduction
Testing environment
- Direwolf 20 1.7.10 1.6.1 with manually updated Buildcraft & Buildcraft compat
- Forge 1.7.10 10.13.4.1448
- FastCraft 1.21
- Buildcraft 7.0.17
- Buildcraft compat 7.0.9
- MobiusCore 1.2.5
- Opis 1.2.5
Steps
- In a new creative flatland world, execute
/opis
to open the Opis window - Observe the idle tick time (e.g. 10ms/t)
- Place down up to 16 Advanced Crafting Tables
- Observe that the tick time grows for each table (e.g. 3ms/t/table)
- Destroy 8 tables, then place a single pink rose in the crafting grid of each of the remaining 8 tables
- Observe that the tick time shrinks back down to idle
Workaround
Place an item (e.g. a pink rose) that has a usage in a crafting recipe, in the crafting grid of any unused ACTs.
Notes
These are based on my limited knowledge of Forge, the crafting system and BuildCraft's codebase; please excuse any errors
- I theorize that a blank crafting recipie causes the ACT to iterate through every single recipe on every update tick. I propose that it should refrain from searching if the grid is empty, or search only when the grid has been changed
- I realize that the ACT is marked as "Deprecated" in NEI. However, I still felt it worth reporting this issue for the benefit of other BC users who may not be aware it is deprecated and in case this issue exists in other BC machinery
whow, this is one of the best, most investigated bug reports i've seen, well done
@RoyCurtis - The ACT will most likely be removed in BuildCraft 7.1, though! I don't see a point in pursuing fixes to a part of code which is going to die soon.
Anyway, your bug report is awesome and you should feel proud. If there's an easy fix to this I might implement it, if not - oh well.
Thank you :) Again, I noticed it was marked deprecated but I figured I would report it anyway.
Regardless, the workaround is easy enough, so perhaps it is indeed not necessary to fix, only document.
@RoyCurtis - a better idea would be to make the ACT only update the recipe if the crafting grid changes. This would effectively nullify it - and that's what the Auto Workbench does, but I have no idea how feasible it is.
(The Recipe Packager and Stamper are even moreso different)