Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to a unified file system, and many related optimisations #338

Merged
merged 16 commits into from
Aug 4, 2023

Conversation

AlexIIL
Copy link
Contributor

@AlexIIL AlexIIL commented Jul 31, 2023

Api changes:

  • Added ExtendedFileSystem / ExtendedFiles . mount(COPY_ON_WRITE or READ_ONLY)` (Currently there's no way to create filesystems with this - so mods can only read file mounts. Also, this only works for files)
  • Added ExtendedFileSystem / ExtendedFiles . isMountedFile, isCopyOnWrite
  • Added FasterFiles / FasterFileSystem . copy

General changes:

  • Added QuiltMapFileSystem, an abstract base type for QuiltZipFileSystem, QuiltMemoryFileSystem, and QuiltUnifiedFileSystem
  • Added QuiltUnifiedFileSystem, a filesystem which supports files of multiple types at once. (Unlike zip file system or memory file system which were a bit more limited)
  • QuiltZipFileSystem can now hold zip files in memory if they aren't stored on disk.
  • Deprecated QuiltMemoryFileSystem, since QuiltUnifiedFileSystem replaces it.

This optimises:

  • Close the transform bundle file system a little bit earlier, which means it doesn't keep on using up memory
  • Adds QuiltBasePath.toStringHashCode(), which is faster then QuiltBasePath.toString().hashCode()
  • Changed QuiltClassPath from Map<String, Path> to Map<Path.toString().hashCode(), Path>, which saves a lot of memory at runtime.
    • This also uses a custom map by default, since the keys are just ints and we can store if it's a hash collision in an array in a Path.
    • Ideally we'd probably just use Int2ObjectHashMap, but it's a bit difficult to import those classes directly into loader.
  • QuiltZipCustomCompressedWriter no longer holds the output in memory before writing it to a file, instead writing entries directly to the file as they are compressed.
    • This nearly obsoletes the system property loader.zipfs.use_temp_file, since the transform cache now uses a much smaller amount of memory.
  • The transform cache uses copy-on-write files when building, which saves memory.
  • Now mods are always loaded using QuiltZipFileSystem rather than JIJ mods being held fully in memory with QuiltMemoryFileSystem

@AlexIIL AlexIIL merged commit 084f11b into QuiltMC:develop Aug 4, 2023
1 check passed
@embeddedt
Copy link
Contributor

FYI, running with -Dloader.transform_cache.disable_optimised_compression=true on 0.20.0-beta.7 seems to cause a crash (presumably caused by the changes in this PR):

UnsupportedOperationException: .minecraft/.cache/quilt_loader/transform-cache-client/files.zip does not support file mounts!
	at org.quiltmc.loader.api.ExtendedFiles.mount(ExtendedFiles.java:57)
	at org.quiltmc.loader.impl.discovery.RuntimeModRemapper.lambda$remap$1(RuntimeModRemapper.java:90)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at org.quiltmc.loader.impl.discovery.RuntimeModRemapper.remap(RuntimeModRemapper.java:72)
	at org.quiltmc.loader.impl.transformer.TransformCache.populateTransformCache(TransformCache.java:307)
	at org.quiltmc.loader.impl.transformer.TransformCache.createTransformCache(TransformCache.java:280)
	at org.quiltmc.loader.impl.transformer.TransformCache.populateTransformBundle(TransformCache.java:106)
	at org.quiltmc.loader.impl.QuiltLoaderImpl.setup(QuiltLoaderImpl.java:349)
	at org.quiltmc.loader.impl.QuiltLoaderImpl.load(QuiltLoaderImpl.java:301)
	at org.quiltmc.loader.impl.launch.knot.Knot.init(Knot.java:147)
	at org.quiltmc.loader.impl.launch.knot.Knot.launch(Knot.java:76)
	at org.quiltmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:28)
	at org.prismlauncher.launcher.impl.StandardLauncher.launch(StandardLauncher.java:88)
	at org.prismlauncher.EntryPoint.listen(EntryPoint.java:126)
	at org.prismlauncher.EntryPoint.main(EntryPoint.java:71)

The good news is that 0.20.0-beta.7 does not seem to require that option in order to successfully reach the main menu with the BlanketCon 23 pack on 700MB, unlike earlier releases. So removing it from my JVM arguments is a good enough workaround. :)

@AlexIIL
Copy link
Contributor Author

AlexIIL commented Aug 5, 2023

Woops - I'll fix that soon! Thanks for testing it though - I'm glad to hear that the optimizations worked :)

AlexIIL added a commit that referenced this pull request Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants