- Notifications
You must be signed in to change notification settings - Fork396
WiP Separate the scalalib from the Scala.js library.#4787
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I agree with you overall assessment of what is problematic. Do we have any chance of benchmarking whether it matters?
|
Paradoxically, it improves code size in 2.12 (but worsens it in 2.13). Less things can be inlined as local loops because of that change. IMO it's more for performance:
Yes, but we never had any benchmarks for that.scalajs-benchmark has benchmarks for immutable collections only. |
Feels like we should make |
sjrd commentedFeb 6, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
This way, we have only one implementation for all the versions ofScala. This will become more useful as we make its implementationless trivial.We also rename it back to `UniquenessCache`, which is the name ithas on the JVM, and reinstate the `Null` lower bound on `V`. Therewas no reason to name it differently, and keeping the name may bebetter for binary compatibility (even though it is `private[scala]`and nobody should be using it). It is still different for the keytype, which is fixed to `String`.
They were changed in757949e touse a `js.WrappedArray` by default, instead of an `ArrayBuffer`.Unfortunately, if we want to make the scalalib independent from theScala.js library, this is not something we can do anymore.Ideally, we would override `ArrayBuffer` itself with animplementation based on `js.Array`. We cannot do that eitherbecause its `array` field is protected, and it is not final.
It has specific overrides in `overrides-2.12` and `overrides-2.13`.
…obal`.This is required not to depend on the Scala.js library from`ExecutionContext`.
This removes references to run-time components of`js.WrappedDictionary` that the IR cleaner cannot get rid of.
We use it in the scalalib, so that we do not generate referencesto `toScalaVarArgs`, which uses custom collections from theScala.js library.
It is configured differently than for the javalib. We allowreferences to `scala.*` (obviously) but disallow references to`scala.scalajs.*`. We also avoid rewriting Scala functions to JSfunctions, since that changes the binary APIs.We use a local copy of `sjs.runtime.AnonFunctionN` classes in`scala.runtime.*` to support the codegen of lambdas.
Seescala/improvement-proposals#54
For now this is only the first step: applying the IR cleaner to the scalalib so that it does not refer to the Scala.js library anymore. It is still bundled in the same artifact.
I identified two annoying things:
mutable.Buffer
. Not sure how much good that did.js.Array
-based seq.Other than that, it is a bit of copy-paste, but not too bad.
@gzm0 Opinions on how this looks so far?