BuildCraft|Core

BuildCraft|Core

7M Downloads

Advanced Crafting Table excess tick times in certain circumstances

RoyCurtis opened this issue ยท 5 comments

commented

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

Reproduction

Testing environment

Steps

  1. In a new creative flatland world, execute /opis to open the Opis window
  2. Observe the idle tick time (e.g. 10ms/t)
  3. Place down up to 16 Advanced Crafting Tables
  4. Observe that the tick time grows for each table (e.g. 3ms/t/table)
  5. Destroy 8 tables, then place a single pink rose in the crafting grid of each of the remaining 8 tables
  6. 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
commented

whow, this is one of the best, most investigated bug reports i've seen, well done

commented

@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.

commented

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.

commented

@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)

commented

Thank you for the fix!