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

Optimize ScalaJSModule and cache IRFileCache #2783

Merged
merged 4 commits into from
Sep 26, 2023

Conversation

lolgab
Copy link
Member

@lolgab lolgab commented Sep 25, 2023

This PR contains various optimizations in ScalaJSModule and ScalaJSWorkerImpl.
Changes:

  • Limit the conversions between types and try to propagate the values to the worker to avoid allocations
  • Avoid manually splitting sjsir files and jar files manually and let the Scala.js toolchain do it with PathIRContainer.fromClasspath(runClasspath) passing the full runClasspath
  • Avoid allocating extra lists
  • Cache the IRFileCache.Cache together with the linker so it can be reused in multiple invocations of fastLinkJS

Pull request: #2783

@lolgab lolgab marked this pull request as ready for review September 25, 2023 20:17
@lolgab lolgab requested a review from lefou September 25, 2023 20:18
Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

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

Can you update the PR description to give me a clue?

@lolgab
Copy link
Member Author

lolgab commented Sep 26, 2023

Can you update the PR description to give me a clue?

Done ✅

@@ -150,14 +145,15 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi {
)
else withModuleSplitStyle

StandardImpl.clearableLinker(withOutputPatterns)
val linker = StandardImpl.clearableLinker(withOutputPatterns)
val irFileCache = StandardImpl.irFileCache().newCache
Copy link

Choose a reason for hiding this comment

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

The IRFileCache is capable to cache IR across different linker instances.

So ideally, you have a single instance of irFileCache() in the entire build (and a newCache per linker). This ensures that, for example, the Scala library is loaded only once. Otherwise you might see very high memory usage.

Copy link
Contributor

@sjrd sjrd left a 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, except the additional improvement suggested by @gzm0.

Copy link
Member

@lefou lefou left a 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. Thanks!

@lefou lefou merged commit a9225cc into com-lihaoyi:main Sep 26, 2023
37 checks passed
@lefou lefou added this to the 0.11.5 milestone Sep 26, 2023
@lolgab lolgab deleted the optimize-scalajsmodule branch September 26, 2023 15:21
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.

4 participants