Skip to content

multi Level Cache#5117

Open
ferriarnus wants to merge 4 commits into
SlimeKnights:1.18.2from
ferriarnus:multilevelcache
Open

multi Level Cache#5117
ferriarnus wants to merge 4 commits into
SlimeKnights:1.18.2from
ferriarnus:multilevelcache

Conversation

@ferriarnus

Copy link
Copy Markdown

Adds a second level cache to the melting inventory.
The constructor of a melting module now takes the parent inventory instead of the block entity. If the Block entity is needed, it can be gotten from the inventory.

@ferriarnus

Copy link
Copy Markdown
Author

So testing these changes with a debugger:

1ste slot iron -> Recipe manager look up
1ste slot iron -> Slot cache
2nd slot iron -> Inventory cache
3rd slot gold -> Recipe manger look up
1ste slot gold -> Inventory cache
2nd slot iron -> Slot cache
3rd slot iron -> Recipe look up / 4rd slot gold -> Inventory cache

Looking at these last 2 interactions, for the 2d slot, the slot cache can be used. However it does not set the inventory cache when used, making it so a look up is used later. Should this be changed so using the slot cache also updates the inventory cache? Or am I missing a case? The advantage of this setup is that if instead of the last iron, gold was used in slot 4, the inventory cache would resolve it.

@ferriarnus ferriarnus marked this pull request as ready for review March 21, 2023 09:32
@KnightMiner

KnightMiner commented Mar 21, 2023

Copy link
Copy Markdown
Member

I am not sure the slot cache should override the inventory cache on a cache hit, if they differ, odds are the next item in the smeltery will also be different. Probably best to stick with what we have, as that is simplier.

The important thing is that when finding a new recipe, it is cached at both the slot and inventory level. Additionally, when a slot has a cache miss, it should cache the inventory recipe even if that was a cache hit. Both of these appear to be done already

@KnightMiner KnightMiner added the 1.18 Issue affects 1.18 label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.18 Issue affects 1.18

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants