-
-
Notifications
You must be signed in to change notification settings - Fork 369
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
Cache Scala.js Linker to enable incremental linking #1007
Cache Scala.js Linker to enable incremental linking #1007
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Do you have any numbers or estimates, what the memory consumption of a cached linker is, and how much speed we gain by caching? My initial though was, that a module-local worker should be enough to cache the linker, but it might be wrong.
I don't. Let me run some tests and I'll let you know.
I don't know if that works because it is common to run fastOpt/fullOpt on the same module. |
Thanks for the numbers, @lolgab ! IMHO, this is already some significant memory, given, that we also keep cached Scala compilers and probably other tools around. Maybe, we can keep |
Didn't know about the |
- Use mutable.Map instead of immutable as cache
89b116a
to
0268586
Compare
Thank you! I'm going to merge after the CI has finished. Travis-CI is currently disabled, so I'm going to start the CI job on my fork. |
@lefou All the relevant jobs are green ✅ |
Thank you for the review as usual @lefou 🙂 |
Originated from this message on gitter.im.
This should solve the problem.
This PR caches all the linkers separately. For now there are 12 combinations. But adding new parameters could explode the possibilities even more.
I did this so you can jump between different settings of fastOpt/fullOpt, ModuleKind without losing the incremental linking.