embeddedt / VintageFix

FerriteCore and ModernFix venturing into the stone age of 1.12
https://legacy.curseforge.com/minecraft/mc-mods/vintagefix
Other
66 stars 6 forks source link

Dynamic Resources optimization causes slowdown in JEI/HEI, especially noticeable with vanilla and custom resources #21

Closed jchung01 closed 1 year ago

jchung01 commented 1 year ago

When viewing items in HEI such as scrolling through pages, there is a very noticeable slowdown/freezing clientside as HEI waits for some textures to load. This slowdown is especially evident if there are custom resources in a /resources folder, such as through ResourceLoader, ContentTweaker/BASE, and Roost. I know it should be expected that dynamic resources will affect performance as the game must load resources that were skipped in the loading stages, but the performance impact on HEI seems to be quite high when using custom resources.

Another scenario where this slowdown is noticeable is when you first load the vanilla textures in HEI (around the first 3 pages on large GUI scale) or otherwise have to reload those vanilla textures because (I assume) they are no longer in the cache. This is arguably the more important issue because this means when clearing your search or otherwise showing the first page of vanilla items, if vanilla textures need to reload, the search will be laggy. Spark profile for that here.

Some blocks/items I noticed considerably struggle to load in HEI are ones from the mods Advent of Ascension, DivineRPG, Betweenlands, Ice and Fire, and of course the aforementioned ContentTweaker.

The following Spark profiles give more info on what is causing these slowdowns (profiled while just scrolling through HEI):

The slowdown seems to depend on the number of resources in the folder, as when I have ContentTweaker/BASE enabled but deleted the entire /resources folder, HEI's performance improved compared to having many resources in the folder. I have highlighted what seem to be the biggest offenders in each profile, which seems to be due to calls to java.io.File.getCanonicalPath() or more specifically java.io.WinNTFileSystem.canonicalize(). I am using Windows, so I'm not sure if the performance impact would be different on Linux.

I tested on the modpack Meatballcraft (which is where all the Spark reports are from), which uses ResourceLoader, ContentTweaker, and Roost resources. I also tested briefly on E2EU, which seems to only noticeably lag in the scenario of (re)loading vanilla textures. *Using VintageFix as of commit d4a3a56

embeddedt commented 1 year ago

After doing some research apparently getCanonicalPath is very slow on Windows. I will have to write some mixins to strip it out of the resource pack handling code. Are there other resource loading mods commonly used besides BASE and Resource Loader?

jchung01 commented 1 year ago

Those are the only two I'm aware of, so it'll probably need further testing/profiling later.

MagmaBro123 commented 1 year ago

Load My Resources exists, iirc

embeddedt commented 1 year ago

How is performance with the latest commit?

jchung01 commented 1 year ago

The performance is definitely better and the profiling seems to indicate that getCanonicalPath is no longer an issue. However, there are still consistent dips in performance when loading items, most noticeably from vanilla and ContentTweaker. Here's some new reports, this time with ResourceLoader back in. Still using same modpack Meatballcraft for the first 3 profiles:

It seems like BASE is no longer an issue, and ResourceLoader is better, but from it NormalResourceLoader.getInputStream still accounts for a sizable amount of time (Can especially see this in the third profile). In fact, I'm not sure how ResourceLoader takes up so much time when viewing CT items considering CT textures don't need ResourceLoader to load.

Other observations are a) AbstractResourcePack.locationToName() uses String.format(), which I think can be improved by using concatenation/StringBuilder instead? and b) throwing FileNotFoundException seems to take up some time. LibNine/FluidDrawers, AE2, and Chisel adds model loaders, which makes the performance impact add up further.

One last thing I've noticed is that it seems like browsing through the creative tabs doesn't have this performance impact that HEI has? I'm not sure if it's because of the different way the items are displayed, but I thought that was odd.

embeddedt commented 1 year ago

In fact, I'm not sure how ResourceLoader takes up so much time when viewing CT items considering CT textures don't need ResourceLoader to load.

ResourceLoader's pack might take priority over BASE; they both basically do the same thing.

One last thing I've noticed is that it seems like browsing through the creative tabs doesn't have this performance impact that HEI has? I'm not sure if it's because of the different way the items are displayed, but I thought that was odd.

Creative tabs display far less models on screen at once which puts a lot less pressure on the model system.

embeddedt commented 1 year ago

I've made some improvements to locationToName. Keep in mind this is probably not going to ever be fully fixed because Windows file IO is just slow in general - launch times are universally faster on Linux from my testing. But I will see how much I can mitigate it.

jchung01 commented 1 year ago

That's understandable, I would say the game is pretty playable, considering it's now only really noticeable with CT and vanilla textures. I figured I would just follow up to report my findings. Anyways, thanks for all the great work on this mod and your other mods!

embeddedt commented 1 year ago

Can you try the latest build?

jchung01 commented 1 year ago

Looks like latest build fixes the performance issues! HEI is very smooth now, profile here.

One last thing that I completely missed earlier, something from commit ea6cca8 causes CustomMainMenu/ResourceLoader to not load images on the main menu correctly. 2023-06-08_12 01 49 latest.log from latest build

embeddedt commented 1 year ago

I am pretty sure this is a Meatballcraft issue. It seems to happen on Linux even without VintageFix. Custom Main Menu tries to load a texture by the name menu:meatball.png (because the texture is converted to a resource location internally, which forces it to lower case) but the actual texture on disk is menu:Meatball.png, so it is not found. I suspect this is normally not noticeable on Windows as the canonicalization (which is what I removed) will sometimes silently correct these issues.

To be clear it is not safe to assume the OS ignores the case of files and Meatballcraft should probably rename this to be lowercase.

jchung01 commented 1 year ago

Got it, that indeed seems to be the case as editing the png to be all lowercase has fixed it. Didn't know about the canonicalization part for Windows. Sorry for reporting it on your side!

embeddedt commented 1 year ago

Good to hear; will close as I think my hack of baking all the item models ahead of time has solved the original problem.