search

QuiltMC / quilt-loader

The loader for Quilt mods.
Apache License 2.0
472 stars 87 forks source link

Optimize boot times by not copying to JAR when unnecessary #453

Closed EnnuiL closed 1 month ago

EnnuiL commented 1 month ago

Once upon a time, a commit happened where in order to fix shenanigans, a deal with the devil was done: if you had Fabric Resource Loader API v0 installed, no matter if you had the Quilted variant, you were going to suffer long load times. This was a terrible deal! But every deal has a fix..

This PR optimizes up to ten seconds (or more! I haven't measured properly) of load times by simply not copying the jar unnecessarily every time the game starts, making dev envs snappier and player happier (I am happier lmfao)

This has been tested on Minecraft 1.21.1, 1.20.1, 1.19.4, 1.19.3, 1.19.2 and 1.18.2 in order to determine the bounds as well as whenever QFAPI was a sure way of fixing the crash

EnnuiL commented 1 month ago

Actually? Here's a decent before/after from my dev env: Before: The timings log from Quilt Loader 0.26.4, showing 7 seconds for "temp jar copy" After: The timings log from the pull request's Quilt Loader, showing 0 seconds for "temp jar copy" It was that bad!

EnnuiL commented 1 month ago

Looks great! I'm not super sure how this system works so some more documentation on what it's used for would be nice.

As far as I know, it's used to fix issues like #217, where this is meant to fix expectations about how the filesystem works; it does have the cost though of uh, being slow. I'm not Alex though, I'm just bothered by how slow QLoader was augymlwulamgulym

AlexIIL commented 1 month ago

Thanks for looking at this - I'd like to look into optimizing tmp jar copy if possible, since 7 seconds seems very slow for a single jar?

Either way this change seems good!

AlexIIL commented 1 month ago

...Oh, one thing that did confuse me is that the modIds passed to shouldCopyToJar are only meant to contain "direct" mods, and not provided mods - did QFAPI change how this is provided?

EnnuiL commented 1 month ago

...Oh, one thing that did confuse me is that the modIds passed to shouldCopyToJar are only meant to contain "direct" mods, and not provided mods - did QFAPI change how this is provided?

Nope, the QFAPI provides were kept intact ever since I quiltified QFAPI

EnnuiL commented 1 month ago

!!! It turns out you are right, if something else provides fabric-resource-loader-v0, it won't pass the check; I don't know how I missed that during the tests, but yeah, I guess this fix is more for a QFAPI-less era now

(this means that the additional check can be removed, with the only important one being the MC version check)