Skip to content

Cache ScalaJS linkers for incremental linking #1761

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

Merged
merged 6 commits into from
Jul 20, 2022

Conversation

lolgab
Copy link
Contributor

@lolgab lolgab commented Jul 15, 2022

This PR enables Scala.js incremental linking by caching the linker in memory in a concurrent map.
To avoid wasting memory (since bloop has a global instance which is long running), this avoid caching the release linkers. Also it caches one linker per output path. If you change the JsConfig the previous Linker instance will be garbage collected and a new one will be saved in the map.

It uses a scala.collection.concurrent.TrieMap as a mutable map implementation.
It also stores SoftReferences so if the GC needs memory it can drop a linker to make space for something else.

The implementation was adapted from the one existing in the Mill build tool.

@lolgab lolgab force-pushed the js-incremental-linker branch from 09cd90c to 6e4abdc Compare July 15, 2022 15:37
Avoid caching fullLinkJS linker to save memory
@lolgab lolgab force-pushed the js-incremental-linker branch from 6e4abdc to fc37fc5 Compare July 15, 2022 15:39
val useClosure = input.isFullLinkJS && input.moduleKind != ModuleKindJS.ESModule

val config = StandardConfig()
.withOptimizer(true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.withOptimizer(true) should be used also with fastLinkJS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withOptimizer(true) is the default anyway. ;)

@lolgab lolgab marked this pull request as ready for review July 15, 2022 20:21
@tgodzik tgodzik self-requested a review July 19, 2022 14:44
@@ -51,6 +53,41 @@ object JsBridge {
}
override def trace(t: => Throwable): Unit = logger.trace(t)
}
private object ScalaJSLinker {
private val cache = TrieMap.empty[Path, WeakReference[(JsConfig, Linker)]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WeakReferences are not the best choice for memory-sensitive caches. Use a SoftReference instead.

val useClosure = input.isFullLinkJS && input.moduleKind != ModuleKindJS.ESModule

val config = StandardConfig()
.withOptimizer(true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withOptimizer(true) is the default anyway. ;)

@lolgab lolgab requested a review from sjrd July 20, 2022 12:55
@tgodzik tgodzik merged commit c1fd321 into scalacenter:main Jul 20, 2022
@lolgab lolgab deleted the js-incremental-linker branch July 20, 2022 13:51
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.

None yet

3 participants