Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Draft
sjrd wants to merge7 commits intoscala-js:main
base:main
Choose a base branch
Loading
fromsjrd:separate-scalalib

Conversation

sjrd
Copy link
Member

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:

  • We have to abandon the Scala.js-specific optimization ofmutable.Buffer. Not sure how much good that did.
  • We have to disable the optimization of Scala varargs using ajs.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?

He-Pin reacted with heart emoji
@gzm0
Copy link
Contributor

I agree with you overall assessment of what is problematic. Do we have any chance of benchmarking whether it matters?

  • For vararg optimization I guess it's only a code size concern.
  • For mutable.Buffer maybe some performance benchmarking would be nice :-/

@sjrd
Copy link
MemberAuthor

For vararg optimization I guess it's only a code size concern.

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:js.ArrayWrapper is a lighter wrapper thanArrayOps (which is a wrapper over anArray, itself a wrapper on a JS Array).

For mutable.Buffer maybe some performance benchmarking would be nice :-/

Yes, but we never had any benchmarks for that.scalajs-benchmark has benchmarks for immutable collections only.mutable.Buffer in particular is a bit weird because we decided to (hopefully) optimize itbased on a "it would be nice". If it happened today, I don't think we would make that change like that.

@gzm0
Copy link
Contributor

gzm0 commentedFeb 6, 2023

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: js.ArrayWrapper is a lighter wrapper than ArrayOps (which is a wrapper over an Array, itself a wrapper on a JS Array).

Feels like we should makescala.Array@inline :)

@sjrd
Copy link
MemberAuthor

sjrd commentedFeb 6, 2023
edited
Loading

scala.Array is magic. It doesn't exist as a class in the IR. But we might want to virtualizeArrayValue nodes in the optimizer in the same way that we virtualizeJSArray nodes.

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.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@sjrd@gzm0

[8]ページ先頭

©2009-2025 Movatter.jp