TPS-Expensive auto-crafting
Closed this issue ยท 6 comments
Describe the bug
Ran some profiling while looking at another issue. Auto-crafting calculations appear TPS-expensive.
How can we reproduce this bug or crash?
What Minecraft version is this happening on?
Minecraft 1.21.1
What NeoForge or Fabric version is this happening on?
NeoForge 21.1.148
What Refined Storage version is this happening on?
2.0.0-beta.2
Relevant log output
Primary causes seems to be constantly checking the max amount that we can craft.
We have to check how much we can craft, as something like the interface could request more of a resource than is currently available.
(The interface could require 64 of something, but we can only craft 8. If we don't correct it to only craft 8, the entire calculation would fail due to missing resources and nothing would happen.)
Even if we get rid of that max amount calculation somehow, constantly calculating tasks will be slow too.
There are a few fixes that could be done:
- We could use some cooldown if missing resources constantly happen (see for example
ExternalStorageWorkRate)- This is pretty difficult to do as we are already throttled within the exporter work rate itself.
- We should only calculate the max amount if a request for the initial amount fails.
- We should only calculate the max amount until the amount we requested initially (binary search low until initial requested amount), and not keep going higher.
- This is not necessary anymore if we only calculate the max amount if a request for the initial amount fails.
- Why? For most cases, there will no longer be an unbounded max amount check, but rather a max amount check that will go up to N. Where N is the typical transfer quota of an exporter or interface (around 64 items).
- It will not be unbounded because we know for sure that there are missing resources.
One observation: The autocrafting currently still walks all the crafting trees, even if one already failed. From my understanding, it should be possible to exit early if a subtree cannot be crafted, if we don't need a list of missing resources.
I've built an extreme example of a Mega Exporter from Cable Tiers trying to export 1x1048m Storage Part, with mostly missing resources but with crafting patterns for all previous storage part tiers.
Before: https://spark.lucko.me/VcXIXh3ADU (note that spark says it was only 1 tick but took 20s. Not sure if that's accurate, but I had a timeout of 11 seconds, so it must be somewhat close)
With an early exit: https://spark.lucko.me/BAw2HS3AFD
@raoulvdberge I'm also not sure if I'm seeing this right in the first of the two reports, but does your latest change result in calculating twice if the exporter only needs to craft 1 but can't? (And the first report also shows that simplifying the hashCode of Pattern might make more sense than I thought before)
Oh, good idea! We only need missing resources for previews, so we can exit early for regular plan calculation.
And yes, good observation. In the case of missing items, we will now calculate twice.
Not ideal, but better than the unbounded max amount call we did before.
@SirYwell Do you want to PR the early return for missing resources?
Also open to other suggestions for improvements for this issue.
#1133 is closely related, as it also has to do with automated autocrafting requests.
Fixes we made so far:
- Pass the cancellation token properly so the timeout is respected for automation too.
- Only calculate the max amount if there are missing resources, don't do an unbounded max amount check by default.
- Exit early for crafting plans if there are missing resources from automation.