Tradeshop / TradeShop

Unique, new, powerful Minecraft TradeShop plugin!
https://tradeshop.github.io
Apache License 2.0
25 stars 11 forks source link

2.5.1 #124

Closed KillerOfPie closed 2 years ago

KillerOfPie commented 2 years ago

Pull Request Etiquette

There are several guidelines you should follow in order for your Pull Request to be merged. Replace [ ] with [x] to cross the checkbox.

It is sometimes better to include more changes in a single commit. If you find yourself having an overwhelming amount of commits, you can rebase your branch.

Changes

Rewrite of item collection to solve bugs

KillerOfPie commented 2 years ago

Were the contents of the list returned by getItems() tested?

I did basic testing to make sure it worked and will do more once I update the missing items messages.

KillerOfPie commented 2 years ago

What do you think of adding a Guava Cache for items in DataStorage, they would save normally but we would lower the amount of disk reads we do. Also since they aren't read as ofen or quick as the Hoppers the Caches can max at like 100 and expireAfterAccess at 1-5min?

This may also speed up the HopperLookups since it would allow me to use the ChestLinkage file instead of the cumbersome ShopChest.isShopChest that is slightly faster than pulling from the drive

KillerOfPie commented 2 years ago

@SparklingComet input on this?

What do you think of adding a Guava Cache for items in DataStorage, they would save normally but we would lower the amount of disk reads we do. Also since they aren't read as ofen or quick as the Hoppers the Caches can max at like 100 and expireAfterAccess at 1-5min?

This may also speed up the HopperLookups since it would allow me to use the ChestLinkage file instead of the cumbersome ShopChest.isShopChest that is slightly faster than pulling from the drive

Thinking about this more I think the time and quantities above would be good for caching shops but for the chest linkage data I think changing it to 30-60min and 500 items may be better since it is a small amount of data per item and having those in memory can significantly speed up a lot of things as well as allow us to fully get rid of using Persistent Block Data which should open up backwards compatibility again.

KillerOfPie commented 2 years ago

got bored so I implemented anyways... Going to push to a separate branch so you can review and decide if we should add in or not. See #125 and leave further discussion/review there.

Below is my basic testing for it

The Red lines are the Approximate start/end of the tests

Trade Testing

Test involves entering server, trading 5 times then returning the items for each of 10 self owned shops, then screenshot.

Current 2.5.1 Branch:

image

With new Caching Code:

image

Hopper Testing

Test involves entering world and waiting for mspT(ms per Tick) to stabilize then teleporting to my hopper testing area with 80k hoppers and again waiting for mspT to stabilize. This will test if checking for shops from the storage is faster with the linkage caching.

Current 2.5.1 Branch:

Pre-Teleport Avg mspT: ~3.3ms Post-Teleport Avg mspT: ~78ms Post-Teleport Peak mspT: ~150ms Return Avg mspT: ~2.8ms

Post Test Screenshot: Forgot to hover for peak and looks like it just GC'd, peaks looks to be around 4000mb image

Post Return Screenshot: image

With new Caching Code:

Pre-Teleport Avg mspT: ~3.1ms Post-Teleport Avg mspT: ~ms Post-Teleport Peak mspT: ~500ms+ Return Avg mspT: ~ms

Ended before full Peak was reached. This probably should've been obvious since caching only helps re-accessing the items and the hoppers would have to each read from disk to check since they wouldn't be in the cache. Only option I see around this is keeping all the linkage data in memory(would have to check each world folder for linkage data on startup and load them all into memory which i'm not a fan of since it wouldn't get cleaned out of memory.