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

Use soft reference for shared service caching #793

Closed

Conversation

Luna5ama
Copy link
Contributor

@Luna5ama Luna5ama commented Jan 7, 2023

This would allow JVM garbage collector clear inactive shared services on demand, eg. when heap memory runs out. Making it possible to build large projects with different mc versions (They can't share remapper service). It should achieve similar effects to #772 without breaking functionality.

@modmuss50
Copy link
Member

I don’t think this is quite right as it may no longer share the services between different projects when it makes sense to, this vastly slowing the build.

The 1.1 branch by default creates and closes a service manger per remap task when multi project optimisations are not enabled.

It’s possible this may not be working quite right as I haven’t yet fully tested it. Creating an integration test that has many mc versions may be a good idea?

@Luna5ama
Copy link
Contributor Author

Luna5ama commented Jan 7, 2023

I don’t think this is quite right as it may no longer share the services between different projects when it makes sense to, this vastly slowing the build.

Each PrepareRemapTask calls RemapJarTask.getTinyRemapperService() from its corresponding remap task. It triggers the memorized supplier, runs TinyRemapperService.getOrCreate(this), and stores the reference to the remapper service. It retains this strong reference until the remap task starts running and passes it to the RemapAction. So it won't be cleared accidentally before the remap task kicks in, and it will also not be cleared until the task finish and release the reference.
If multiple remap tasks are sharing the remapper service, they all will retain the reference to the remapper service during the prepare remap task. Therefore they will still be able to share the remapper service.

It’s possible this may not be working quite right as I haven’t yet fully tested it. Creating an integration test that has many mc versions may be a good idea?

Sure that will be great, since the current tests don't have coverage for that.

@modmuss50
Copy link
Member

I have spent a few hours profiling and optimising loom here in this PR here: #796 this allows me to build FabricAPI with `-Xmx2G'.

Loom 1.1 uses a ref counting BuildSharedServiceManager for the remap jar task to close the TinyRemapper instance once its finished.

I think I will cherry pick your test into my branch to help test this.

@modmuss50
Copy link
Member

All of the shared service stuff has been removed in 1.8, this should no longer be an issue.

@modmuss50 modmuss50 closed this Sep 3, 2024
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