From 037e79352f7d7527efa3d4ecdfc0f0001ad90542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Fri, 11 Feb 2022 16:53:20 +0100 Subject: [PATCH 01/20] Towards 1.9.1. --- .../src/main/scala/org/scalajs/ir/ScalaJSVersions.scala | 2 +- project/Build.scala | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala b/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala index e06bbe21c3..ade4a9b81d 100644 --- a/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala +++ b/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala @@ -17,7 +17,7 @@ import java.util.concurrent.ConcurrentHashMap import scala.util.matching.Regex object ScalaJSVersions extends VersionChecks( - current = "1.9.0", + current = "1.9.1-SNAPSHOT", binaryEmitted = "1.8" ) diff --git a/project/Build.scala b/project/Build.scala index b18602a021..4e51ae953d 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -240,8 +240,9 @@ object Build { val packageMinilib = taskKey[File]("Produces the minilib jar.") - val previousVersions = List("1.0.0", "1.0.1", "1.1.0", "1.1.1", - "1.2.0", "1.3.0", "1.3.1", "1.4.0", "1.5.0", "1.5.1", "1.6.0", "1.7.0", "1.7.1", "1.8.0") + val previousVersions = List("1.0.0", "1.0.1", "1.1.0", "1.1.1", "1.2.0", + "1.3.0", "1.3.1", "1.4.0", "1.5.0", "1.5.1", "1.6.0", "1.7.0", "1.7.1", + "1.8.0", "1.9.0") val previousVersion = previousVersions.last val previousBinaryCrossVersion = CrossVersion.binaryWith("sjs1_", "") From 59c14f17b3c7b7cd56f7eec40152466a70fb43b6 Mon Sep 17 00:00:00 2001 From: danicheg Date: Wed, 2 Mar 2022 01:09:17 +0300 Subject: [PATCH 02/20] Highlight the using nowarnGlobalExecutionContext option within Scala 3 --- .../src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala | 2 +- .../nscplugin/test/GlobalExecutionContextWarnTest.scala | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala b/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala index dc2f712df4..a4ed63cd5e 100644 --- a/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala +++ b/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala @@ -393,7 +393,7 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G) | |If you do not care about macrotask fairness, you can silence this warning by: |- Adding @nowarn("cat=other") (Scala >= 2.13.x only) - |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option + |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option (Scala < 3.x.y only) |- Using scala.scalajs.concurrent.JSExecutionContext.queue | (the implementation of ExecutionContext.global in Scala.js) directly. | diff --git a/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala b/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala index daef9b1add..1fd1333eb1 100644 --- a/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala +++ b/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala @@ -42,7 +42,7 @@ class GlobalExecutionContextWarnTest extends DirectTest with TestHelpers { | |If you do not care about macrotask fairness, you can silence this warning by: |- Adding @nowarn("cat=other") (Scala >= 2.13.x only) - |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option + |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option (Scala < 3.x.y only) |- Using scala.scalajs.concurrent.JSExecutionContext.queue | (the implementation of ExecutionContext.global in Scala.js) directly. | @@ -77,7 +77,7 @@ class GlobalExecutionContextWarnTest extends DirectTest with TestHelpers { | |If you do not care about macrotask fairness, you can silence this warning by: |- Adding @nowarn("cat=other") (Scala >= 2.13.x only) - |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option + |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option (Scala < 3.x.y only) |- Using scala.scalajs.concurrent.JSExecutionContext.queue | (the implementation of ExecutionContext.global in Scala.js) directly. | From 6b0f043adda7c0bda2c2839a099f6aa951f00fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 23 Feb 2022 12:13:18 +0100 Subject: [PATCH 03/20] Floating point bits polyfill implementations without js.Math functions. When encoding, to compute the exponent, we perform a binary search inside an array of all exact powers of 2. The loop executes exactly 11 times for Doubles, and 8 times for Floats. It replaces a call to `log`, `floor` and `pow`, in addition to two conditions to fix up `e` and `significand`. When decoding, we also use the same array to get the power of 2 via direct indexing, rather than calling `pow`. --- .../scala/java/lang/FloatingPointBits.scala | 220 ++++++++++-------- project/Build.scala | 2 +- project/MiniLib.scala | 1 - 3 files changed, 128 insertions(+), 95 deletions(-) diff --git a/javalanglib/src/main/scala/java/lang/FloatingPointBits.scala b/javalanglib/src/main/scala/java/lang/FloatingPointBits.scala index 66472259c6..359991af0e 100644 --- a/javalanglib/src/main/scala/java/lang/FloatingPointBits.scala +++ b/javalanglib/src/main/scala/java/lang/FloatingPointBits.scala @@ -73,6 +73,28 @@ private[lang] object FloatingPointBits { private val highOffset = if (areTypedArraysBigEndian) 0 else 1 private val lowOffset = if (areTypedArraysBigEndian) 1 else 0 + private val floatPowsOf2: js.Array[scala.Double] = + if (areTypedArraysSupported) null + else makePowsOf2(len = 1 << 8, java.lang.Float.MIN_NORMAL.toDouble) + + private val doublePowsOf2: js.Array[scala.Double] = + if (areTypedArraysSupported) null + else makePowsOf2(len = 1 << 11, java.lang.Double.MIN_NORMAL) + + private def makePowsOf2(len: Int, minNormal: scala.Double): js.Array[scala.Double] = { + val r = new js.Array[scala.Double](len) + r(0) = 0.0 + var i = 1 + var next = minNormal + while (i != len - 1) { + r(i) = next + i += 1 + next *= 2 + } + r(len - 1) = scala.Double.PositiveInfinity + r + } + /** Hash code of a number (excluding Longs). * * Because of the common encoding for integer and floating point values, @@ -166,29 +188,42 @@ private[lang] object FloatingPointBits { private def intBitsToFloatPolyfill(bits: Int): scala.Double = { val ebits = 8 val fbits = 23 - val s = bits < 0 + val sign = (bits >> 31) | 1 // -1 or 1 val e = (bits >> fbits) & ((1 << ebits) - 1) val f = bits & ((1 << fbits) - 1) - decodeIEEE754(ebits, fbits, scala.Float.MinPositiveValue, s, e, f) + decodeIEEE754(ebits, fbits, floatPowsOf2, scala.Float.MinPositiveValue, sign, e, f) } private def floatToIntBitsPolyfill(value: scala.Double): Int = { + // Some constants val ebits = 8 val fbits = 23 - val sef = encodeIEEE754(ebits, fbits, Float.MIN_NORMAL, scala.Float.MinPositiveValue, value) - (if (sef.s) 0x80000000 else 0) | (sef.e << fbits) | rawToInt(sef.f) + + // Determine sign bit and compute the absolute value av + val sign = if (value < 0.0 || (value == 0.0 && 1.0 / value < 0.0)) -1 else 1 + val s = sign & scala.Int.MinValue + val av = sign * value + + // Compute e and f + val avr = forceFround(av) + val powsOf2 = this.floatPowsOf2 // local cache + val e = encodeIEEE754Exponent(ebits, powsOf2, avr) + val f = encodeIEEE754MantissaBits(ebits, fbits, powsOf2, scala.Float.MinPositiveValue.toDouble, avr, e) + + // Encode + s | (e << fbits) | rawToInt(f) } private def longBitsToDoublePolyfill(bits: scala.Long): scala.Double = { val ebits = 11 val fbits = 52 - val hifbits = fbits-32 + val hifbits = fbits - 32 val hi = (bits >>> 32).toInt val lo = Utils.toUint(bits.toInt) - val s = hi < 0 + val sign = (hi >> 31) | 1 // -1 or 1 val e = (hi >> hifbits) & ((1 << ebits) - 1) val f = (hi & ((1 << hifbits) - 1)).toDouble * 0x100000000L.toDouble + lo - decodeIEEE754(ebits, fbits, scala.Double.MinPositiveValue, s, e, f) + decodeIEEE754(ebits, fbits, doublePowsOf2, scala.Double.MinPositiveValue, sign, e, f) } @noinline @@ -197,124 +232,123 @@ private[lang] object FloatingPointBits { @inline private def doubleToLongBitsPolyfillInline(value: scala.Double): scala.Long = { + // Some constants val ebits = 11 val fbits = 52 - val hifbits = fbits-32 - val sef = encodeIEEE754(ebits, fbits, Double.MIN_NORMAL, scala.Double.MinPositiveValue, value) - val hif = rawToInt(sef.f / 0x100000000L.toDouble) - val hi = (if (sef.s) 0x80000000 else 0) | (sef.e << hifbits) | hif - val lo = rawToInt(sef.f) + val hifbits = fbits - 32 + + // Determine sign bit and compute the absolute value av + val sign = if (value < 0.0 || (value == 0.0 && 1.0 / value < 0.0)) -1 else 1 + val s = sign & scala.Int.MinValue + val av = sign * value + + // Compute e and f + val powsOf2 = this.doublePowsOf2 // local cache + val e = encodeIEEE754Exponent(ebits, powsOf2, av) + val f = encodeIEEE754MantissaBits(ebits, fbits, powsOf2, scala.Double.MinPositiveValue, av, e) + + // Encode + val hi = s | (e << hifbits) | rawToInt(f / 0x100000000L.toDouble) + val lo = rawToInt(f) (hi.toLong << 32) | (lo.toLong & 0xffffffffL) } - @inline private def decodeIEEE754(ebits: Int, fbits: Int, - minPositiveValue: scala.Double, s: scala.Boolean, e: Int, - f: scala.Double): scala.Double = { - - import Math.pow + @inline + private def decodeIEEE754(ebits: Int, fbits: Int, + powsOf2: js.Array[scala.Double], minPositiveValue: scala.Double, + sign: scala.Int, e: Int, f: scala.Double): scala.Double = { // Some constants - val bias = (1 << (ebits - 1)) - 1 val specialExponent = (1 << ebits) - 1 val twoPowFbits = (1L << fbits).toDouble - val absResult = if (e == specialExponent) { + if (e == specialExponent) { // Special if (f == 0.0) - scala.Double.PositiveInfinity + sign * scala.Double.PositiveInfinity else scala.Double.NaN } else if (e > 0) { // Normalized - pow(2, e - bias) * (1 + f / twoPowFbits) + sign * powsOf2(e) * (1 + f / twoPowFbits) } else { // Subnormal - f * minPositiveValue + sign * f * minPositiveValue } + } - if (s) -absResult else absResult + /** Force rounding of `av` to fit in 32 bits (this is a manual `fround`). + * + * `av` must not be negative, i.e., `av < 0.0` must be false (it can be + * `NaN` or `Infinity`). + * + * When we use strict-float semantics, this is redundant, because the input + * came from a `Float` and is therefore guaranteed to be rounded already. + * However, here we don't know whether we use strict floats semantics or + * not, so we must always do it. This is not a big deal because, if this + * code is called, then any operation on `Float`s is calling the same code + * from the `CoreJSLib`, so doing one more such operation for + * `floatToIntBits` is negligible. + * + * TODO Remove this when we get rid of non-strict float semantics altogether. + */ + @inline + private def forceFround(av: scala.Double): scala.Double = { + // See the `fround` polyfill in CoreJSLib + val overflowThreshold = 3.4028235677973366e38 + val normalThreshold = 1.1754943508222875e-38 + if (av >= overflowThreshold) { + scala.Double.PositiveInfinity + } else if (av >= normalThreshold) { + val p = av * 536870913.0 // pow(2, 29) + 1 + p + (av - p) + } else { + val roundingFactor = scala.Double.MinPositiveValue / scala.Float.MinPositiveValue.toDouble + (av * roundingFactor) / roundingFactor + } } - @inline private def encodeIEEE754(ebits: Int, fbits: Int, - minNormal: scala.Double, minPositiveValue: scala.Double, - v: scala.Double): EncodeIEEE754Result = { + private def encodeIEEE754Exponent(ebits: Int, + powsOf2: js.Array[scala.Double], av: scala.Double): Int = { - import js.Math.{floor, log, pow} + /* Binary search of `av` inside `powsOf2`. + * There are exactly `ebits` iterations of this loop (11 for Double, 8 for Float). + */ + var eMin = 0 + var eMax = 1 << ebits + while (eMin + 1 < eMax) { + val e = (eMin + eMax) >> 1 + if (av < powsOf2(e)) // false when av is NaN + eMax = e + else + eMin = e + } + eMin + } + + @inline + private def encodeIEEE754MantissaBits(ebits: Int, fbits: Int, + powsOf2: js.Array[scala.Double], minPositiveValue: scala.Double, + av: scala.Double, e: Int): scala.Double = { // Some constants - val bias = (1 << (ebits - 1)) - 1 val specialExponent = (1 << ebits) - 1 val twoPowFbits = (1L << fbits).toDouble - val highestOneBitOfFbits = (1L << (fbits - 1)).toDouble - val LN2 = 0.6931471805599453 - - if (Double.isNaN(v)) { - // http://dev.w3.org/2006/webapi/WebIDL/#es-type-mapping - new EncodeIEEE754Result(false, specialExponent, highestOneBitOfFbits) - } else if (Double.isInfinite(v)) { - new EncodeIEEE754Result(v < 0, specialExponent, 0.0) - } else if (v == 0.0) { - new EncodeIEEE754Result(1 / v == scala.Double.NegativeInfinity, 0, 0.0) + + if (e == specialExponent) { + if (av != av) + (1L << (fbits - 1)).toDouble // NaN + else + 0.0 // Infinity } else { - val s = v < 0 - val av = if (s) -v else v - - if (av >= minNormal) { - // Normalized - - var e = rawToInt(floor(log(av) / LN2)) - if (e > 1023) - e = 1023 - var significand = av / pow(2, e) - - /* #2911 then #4433: When av is very close to a power of 2 (e.g., - * 9007199254740991.0 == 2^53 - 1), `log(av) / LN2` will already round - * *up* to an `e` which is 1 too high, or *down* to an `e` which is 1 - * too low. The `floor()` afterwards comes too late to fix that. - * We now adjust `e` and `significand` to make sure that `significand` - * is in the range [1.0, 2.0) - */ - if (significand < 1.0) { - e -= 1 - significand *= 2 - } else if (significand >= 2.0) { - e += 1 - significand /= 2 - } - - // Compute the stored bits of the mantissa (without the implicit leading '1') - var f = roundToEven((significand - 1.0) * twoPowFbits) - if (f == twoPowFbits) { // can happen because of the round-to-even - e += 1 - f = 0 - } - - // Introduce the bias into `e` - e += bias - - if (e > 2 * bias) { - // Overflow - e = specialExponent - f = 0 - } - - new EncodeIEEE754Result(s, e, f) - } else { - // Subnormal - new EncodeIEEE754Result(s, 0, roundToEven(av / minPositiveValue)) - } + if (e == 0) + av / minPositiveValue // Subnormal + else + ((av / powsOf2(e)) - 1.0) * twoPowFbits // Normal } } @inline private def rawToInt(x: scala.Double): Int = (x.asInstanceOf[js.Dynamic] | 0.asInstanceOf[js.Dynamic]).asInstanceOf[Int] - @inline private def roundToEven(n: scala.Double): scala.Double = - (n * scala.Double.MinPositiveValue) / scala.Double.MinPositiveValue - - // Cannot use tuples in the javalanglib - @inline - private final class EncodeIEEE754Result(val s: scala.Boolean, val e: Int, - val f: scala.Double) - } diff --git a/project/Build.scala b/project/Build.scala index 4e51ae953d..b6d1fe03c6 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1735,7 +1735,7 @@ object Build { case Default2_12ScalaVersion => Some(ExpectedSizes( - fastLink = 783000 to 784000, + fastLink = 784000 to 785000, fullLink = 150000 to 151000, fastLinkGz = 92000 to 93000, fullLinkGz = 37000 to 38000, diff --git a/project/MiniLib.scala b/project/MiniLib.scala index 4fd689fa90..2b873dc5f3 100644 --- a/project/MiniLib.scala +++ b/project/MiniLib.scala @@ -24,7 +24,6 @@ object MiniLib { "String", "FloatingPointBits", - "FloatingPointBits$EncodeIEEE754Result", "Throwable", "StackTrace", From 6555a23174b2c8f96dc1d7c310f354ed9deb84e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Fri, 2 Aug 2019 11:50:04 +0200 Subject: [PATCH 04/20] Avoid using `scala.concurrent.duration` in the javalib. --- javalib/src/main/scala/java/util/Timer.scala | 12 +++++------- javalib/src/main/scala/java/util/TimerTask.scala | 5 ++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/javalib/src/main/scala/java/util/Timer.scala b/javalib/src/main/scala/java/util/Timer.scala index 075dffd44d..2873c481e3 100644 --- a/javalib/src/main/scala/java/util/Timer.scala +++ b/javalib/src/main/scala/java/util/Timer.scala @@ -12,8 +12,6 @@ package java.util -import scala.concurrent.duration._ - class Timer() { private[util] var canceled: Boolean = false @@ -49,7 +47,7 @@ class Timer() { private def scheduleOnce(task: TimerTask, delay: Long): Unit = { acquire(task) - task.timeout(delay.millis) { + task.timeout(delay) { task.scheduledOnceAndStarted = true task.doRun() } @@ -72,13 +70,13 @@ class Timer() { private def schedulePeriodically( task: TimerTask, delay: Long, period: Long): Unit = { acquire(task) - task.timeout(delay.millis) { + task.timeout(delay) { def loop(): Unit = { val startTime = System.nanoTime() task.doRun() val endTime = System.nanoTime() val duration = (endTime - startTime) / 1000000 - task.timeout((period - duration).millis) { + task.timeout(period - duration) { loop() } } @@ -102,7 +100,7 @@ class Timer() { private def scheduleFixed( task: TimerTask, delay: Long, period: Long): Unit = { acquire(task) - task.timeout(delay.millis) { + task.timeout(delay) { def loop(scheduledTime: Long): Unit = { task.doRun() val nextScheduledTime = scheduledTime + period @@ -112,7 +110,7 @@ class Timer() { loop(nextScheduledTime) } else { // Re-run after a timeout. - task.timeout((nextScheduledTime - nowTime).millis) { + task.timeout(nextScheduledTime - nowTime) { loop(nextScheduledTime) } } diff --git a/javalib/src/main/scala/java/util/TimerTask.scala b/javalib/src/main/scala/java/util/TimerTask.scala index c775afa4fe..959d206f53 100644 --- a/javalib/src/main/scala/java/util/TimerTask.scala +++ b/javalib/src/main/scala/java/util/TimerTask.scala @@ -12,7 +12,6 @@ package java.util -import scala.concurrent.duration.FiniteDuration import scala.scalajs.js.timers._ import scala.scalajs.js.timers.SetTimeoutHandle @@ -41,9 +40,9 @@ abstract class TimerTask { def scheduledExecutionTime(): Long = lastScheduled - private[util] def timeout(delay: FiniteDuration)(body: => Unit): Unit = { + private[util] def timeout(delay: Long)(body: => Unit): Unit = { if (!canceled) { - handle = setTimeout(delay)(body) + handle = setTimeout(delay.toDouble)(body) } } From 7d7a62193143d5f9e8e6966e3943e2745becdf84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Fri, 2 Aug 2019 14:53:59 +0200 Subject: [PATCH 05/20] Avoid using Scala's Option in `Reader`, `Writer` and `URI`. --- javalib/src/main/scala/java/io/Reader.scala | 16 +++++++++------- javalib/src/main/scala/java/io/Writer.scala | 16 +++++++++------- javalib/src/main/scala/java/net/URI.scala | 4 ++-- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/javalib/src/main/scala/java/io/Reader.scala b/javalib/src/main/scala/java/io/Reader.scala index ecd53f732d..d2733b550c 100644 --- a/javalib/src/main/scala/java/io/Reader.scala +++ b/javalib/src/main/scala/java/io/Reader.scala @@ -16,13 +16,15 @@ import java.nio.CharBuffer import scala.annotation.tailrec -abstract class Reader private[this] (_lock: Option[Object]) - extends Readable with Closeable { - - protected val lock = _lock.getOrElse(this) - - protected def this(lock: Object) = this(Some(lock)) - protected def this() = this(None) +abstract class Reader() extends Readable with Closeable { + protected var lock: Object = this + + protected def this(lock: Object) = { + this() + if (lock eq null) + throw new NullPointerException() + this.lock = lock + } def read(target: CharBuffer): Int = { if (!target.hasRemaining()) 0 diff --git a/javalib/src/main/scala/java/io/Writer.scala b/javalib/src/main/scala/java/io/Writer.scala index 92f9de2305..4dd6e1bd0d 100644 --- a/javalib/src/main/scala/java/io/Writer.scala +++ b/javalib/src/main/scala/java/io/Writer.scala @@ -12,13 +12,15 @@ package java.io -abstract class Writer private[this] (_lock: Option[Object]) extends - Appendable with Closeable with Flushable { - - protected val lock = _lock.getOrElse(this) - - protected def this(lock: Object) = this(Some(lock)) - protected def this() = this(None) +abstract class Writer() extends Appendable with Closeable with Flushable { + protected var lock: Object = this + + protected def this(lock: Object) = { + this() + if (lock eq null) + throw new NullPointerException() + this.lock = lock + } def write(c: Int): Unit = write(Array(c.toChar)) diff --git a/javalib/src/main/scala/java/net/URI.scala b/javalib/src/main/scala/java/net/URI.scala index b8bff75065..553dd72e8b 100644 --- a/javalib/src/main/scala/java/net/URI.scala +++ b/javalib/src/main/scala/java/net/URI.scala @@ -31,9 +31,9 @@ final class URI(origStr: String) extends Serializable with Comparable[URI] { * This is a local val for the primary constructor. It is a val, * since we'll set it to null after initializing all fields. */ - private[this] var _fld = Option(URI.uriRe.exec(origStr)).getOrElse { + private[this] var _fld: RegExp.ExecResult = URI.uriRe.exec(origStr) + if (_fld == null) throw new URISyntaxException(origStr, "Malformed URI") - } private val _isAbsolute = fld(AbsScheme).isDefined private val _isOpaque = fld(AbsOpaquePart).isDefined From 617fc8e73f9a97d0e3b5b1fb623cbffd1f06191a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 8 Dec 2021 17:25:02 +0100 Subject: [PATCH 06/20] Refactor URLDecoder not to use any local lazy val. These were the only place in the javalib where we used local lazy vals. Since their compilation scheme involves classes of `scala.runtime.*` (unlike field lazy vals), we avoid them. --- .../src/main/scala/java/net/URLDecoder.scala | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/javalib/src/main/scala/java/net/URLDecoder.scala b/javalib/src/main/scala/java/net/URLDecoder.scala index a3f8674f91..68e11e617d 100644 --- a/javalib/src/main/scala/java/net/URLDecoder.scala +++ b/javalib/src/main/scala/java/net/URLDecoder.scala @@ -12,36 +12,37 @@ package java.net +import scala.scalajs.js + import java.io.UnsupportedEncodingException import java.nio.{CharBuffer, ByteBuffer} -import java.nio.charset.{Charset, MalformedInputException} +import java.nio.charset.{Charset, CharsetDecoder} object URLDecoder { @Deprecated - def decode(s: String): String = decodeImpl(s, Charset.defaultCharset()) + def decode(s: String): String = decodeImpl(s, () => Charset.defaultCharset()) def decode(s: String, enc: String): String = { - /* An exception is thrown only if the - * character encoding needs to be consulted - */ - lazy val charset = { + decodeImpl(s, { () => + /* An exception is thrown only if the + * character encoding needs to be consulted + */ if (!Charset.isSupported(enc)) throw new UnsupportedEncodingException(enc) else Charset.forName(enc) - } - - decodeImpl(s, charset) + }) } - private def decodeImpl(s: String, charset: => Charset): String = { + private def decodeImpl(s: String, getCharset: js.Function0[Charset]): String = { val len = s.length - lazy val charsetDecoder = charset.newDecoder() - - lazy val byteBuffer = ByteBuffer.allocate(len / 3) val charBuffer = CharBuffer.allocate(len) + // For charset-based decoding + var decoder: CharsetDecoder = null + var byteBuffer: ByteBuffer = null + def throwIllegalHex() = { throw new IllegalArgumentException( "URLDecoder: Illegal hex characters in escape (%) pattern") @@ -58,10 +59,13 @@ object URLDecoder { throwIllegalHex() case '%' => - val decoder = charsetDecoder - val buffer = byteBuffer - buffer.clear() - decoder.reset() + if (decoder == null) { // equivalent to `byteBuffer == null` + decoder = getCharset().newDecoder() + byteBuffer = ByteBuffer.allocate(len / 3) + } else { + byteBuffer.clear() + decoder.reset() + } while (i + 3 <= len && s.charAt(i) == '%') { val c1 = Character.digit(s.charAt(i + 1), 16) @@ -70,12 +74,12 @@ object URLDecoder { if (c1 < 0 || c2 < 0) throwIllegalHex() - buffer.put(((c1 << 4) + c2).toByte) + byteBuffer.put(((c1 << 4) + c2).toByte) i += 3 } - buffer.flip() - val decodeResult = decoder.decode(buffer, charBuffer, true) + byteBuffer.flip() + val decodeResult = decoder.decode(byteBuffer, charBuffer, true) val flushResult = decoder.flush(charBuffer) if (decodeResult.isError() || flushResult.isError()) From 99cb95aa166ace6630709e53c246108598716830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 3 Jun 2020 17:50:56 +0200 Subject: [PATCH 07/20] Improve the error messages reported by the IR cleaner. --- project/Build.scala | 4 +++- project/JavalibIRCleaner.scala | 12 +++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/project/Build.scala b/project/Build.scala index b6d1fe03c6..4beac24972 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -562,6 +562,8 @@ object Build { val outputDir = crossTarget.value / "cleaned-classes" + val irCleaner = new JavalibIRCleaner((LocalRootProject / baseDirectory).value.toURI()) + val libFileMappings = (PathFinder(prevProducts) ** "*.sjsir") .pair(Path.rebase(prevProducts, outputDir)) @@ -578,7 +580,7 @@ object Build { IO.delete(outputDir) IO.createDirectory(outputDir) - JavalibIRCleaner.cleanIR(dependencyFiles, libFileMappings, s.log) + irCleaner.cleanIR(dependencyFiles, libFileMappings, s.log) } ((dependencyFiles ++ libFileMappings.map(_._1)).toSet) Seq(outputDir) diff --git a/project/JavalibIRCleaner.scala b/project/JavalibIRCleaner.scala index 670b1a0cc6..6893b954ab 100644 --- a/project/JavalibIRCleaner.scala +++ b/project/JavalibIRCleaner.scala @@ -7,6 +7,7 @@ import org.scalajs.ir.Trees._ import org.scalajs.ir.Types._ import java.io._ +import java.net.URI import java.nio.file.Files import scala.collection.mutable @@ -32,7 +33,7 @@ import sbt.{Logger, MessageOnlyException} * Afterwards, we check that the IR does not contain any reference to classes * under the `scala.*` package. */ -object JavalibIRCleaner { +final class JavalibIRCleaner(baseDirectoryURI: URI) { private val JavaIOSerializable = ClassName("java.io.Serializable") private val ScalaSerializable = ClassName("scala.Serializable") @@ -72,7 +73,7 @@ object JavalibIRCleaner { case JSClass | JSModuleClass => errorManager.reportError( - s"found non-native JS class ${tree.className}")(tree.pos) + s"found non-native JS class ${tree.className.nameString}")(tree.pos) } } @@ -91,7 +92,8 @@ object JavalibIRCleaner { private var _errorCount: Int = 0 def reportError(msg: String)(implicit pos: Position): Unit = { - logger.error(s"$msg at $pos") + val fileStr = baseDirectoryURI.relativize(pos.source).toString + logger.error(s"$msg at $fileStr:${pos.line}:${pos.column}") _errorCount += 1 } @@ -244,7 +246,7 @@ object JavalibIRCleaner { case t: ClassOf => if (transformTypeRef(t.typeRef) != t.typeRef) - reportError(s"illegal Class(${t.typeRef})") + reportError(s"illegal ClassOf(${t.typeRef})") t case t => @@ -375,7 +377,7 @@ object JavalibIRCleaner { } private def reportError(msg: String)(implicit pos: Position): Unit = { - errorManager.reportError(s"$msg in $enclosingClassName") + errorManager.reportError(s"$msg in ${enclosingClassName.nameString}") } } } From dd857943ef14545f524c4a83dc0008050ae9174a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 8 Dec 2021 16:56:05 +0100 Subject: [PATCH 08/20] Remove unnecessary changed-based "optimization" in the IR cleaner. --- project/JavalibIRCleaner.scala | 41 +++++++++++++--------------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/project/JavalibIRCleaner.scala b/project/JavalibIRCleaner.scala index 6893b954ab..cc570af9cf 100644 --- a/project/JavalibIRCleaner.scala +++ b/project/JavalibIRCleaner.scala @@ -137,35 +137,24 @@ final class JavalibIRCleaner(baseDirectoryURI: URI) { def cleanClassDef(tree: ClassDef): ClassDef = { import tree._ - val preprocessedTree = { - var changed = false - - // Preprocess the super interface list - val newInterfaces = transformInterfaceList(interfaces) - if (newInterfaces ne interfaces) - changed = true - - /* Remove the `private def writeReplace__O` generated by scalac 2.13+ - * in the companion of serializable classes. - */ - val newMemberDefs = memberDefs.filter { - case MethodDef(_, MethodIdent(`writeReplaceMethodName`), _, _, _, _) => - changed = true - false - case _ => - true - } + // Preprocess the super interface list + val newInterfaces = transformInterfaceList(interfaces) - if (changed) { - ClassDef(name, originalName, kind, jsClassCaptures, superClass, - newInterfaces, jsSuperClass, jsNativeLoadSpec, newMemberDefs, - topLevelExportDefs)( - optimizerHints)(pos) - } else { - tree - } + /* Remove the `private def writeReplace__O` generated by scalac 2.13+ + * in the companion of serializable classes. + */ + val newMemberDefs = memberDefs.filter { + case MethodDef(_, MethodIdent(`writeReplaceMethodName`), _, _, _, _) => + false + case _ => + true } + val preprocessedTree = ClassDef(name, originalName, kind, jsClassCaptures, + superClass, newInterfaces, jsSuperClass, jsNativeLoadSpec, + newMemberDefs, topLevelExportDefs)( + optimizerHints)(pos) + validateClassName(preprocessedTree.name.name) for (superClass <- preprocessedTree.superClass) validateClassName(superClass.name) From 56d3bb0043538d9682efb1151d63ce5d5392c5aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Wed, 8 Dec 2021 17:34:31 +0100 Subject: [PATCH 09/20] Deduplicate errors in the IR cleaner. --- project/JavalibIRCleaner.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/project/JavalibIRCleaner.scala b/project/JavalibIRCleaner.scala index cc570af9cf..5e6f4fc913 100644 --- a/project/JavalibIRCleaner.scala +++ b/project/JavalibIRCleaner.scala @@ -89,12 +89,16 @@ final class JavalibIRCleaner(baseDirectoryURI: URI) { } private final class ErrorManager(logger: Logger) { + private val seenErrors = mutable.Set.empty[String] private var _errorCount: Int = 0 def reportError(msg: String)(implicit pos: Position): Unit = { val fileStr = baseDirectoryURI.relativize(pos.source).toString - logger.error(s"$msg at $fileStr:${pos.line}:${pos.column}") - _errorCount += 1 + val fullMessage = s"$msg at $fileStr:${pos.line}:${pos.column}" + if (seenErrors.add(fullMessage)) { + logger.error(fullMessage) + _errorCount += 1 + } } def hasErrors: Boolean = _errorCount != 0 From 93378365380c1b24f2f4f824ff2406a13be4671f Mon Sep 17 00:00:00 2001 From: David Barri Date: Tue, 8 Mar 2022 16:18:33 +1100 Subject: [PATCH 10/20] Introduce `IsUsed` as a `Boolean` replacement in `OptimizerCore` --- .../frontend/optimizer/OptimizerCore.scala | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index 4cd8f79d9a..04137d4703 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -427,7 +427,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { val (newName, newOriginalName) = freshLocalName(name, originalName, mutable = false) val localDef = LocalDef(RefinedType(AnyType), mutable = false, - ReplaceWithVarRef(newName, newSimpleState(true), None)) + ReplaceWithVarRef(newName, newSimpleState(Used), None)) val newBody = { val bodyScope = scope.withEnv(scope.env.withLocalDef(name, localDef)) transformStat(body)(bodyScope) @@ -440,7 +440,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { val (newName, newOriginalName) = freshLocalName(name, originalName, mutable = false) val localDef = LocalDef(RefinedType(AnyType), true, - ReplaceWithVarRef(newName, newSimpleState(true), None)) + ReplaceWithVarRef(newName, newSimpleState(Used), None)) val newHandler = { val handlerScope = scope.withEnv(scope.env.withLocalDef(name, localDef)) transform(handler, isStat)(handlerScope) @@ -919,7 +919,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { withNewLocalDefs(captureBindings) { (captureLocalDefs, cont1) => val replacement = TentativeClosureReplacement( captureParams, params, body, captureLocalDefs, - alreadyUsed = newSimpleState(false), cancelFun) + alreadyUsed = newSimpleState(Unused), cancelFun) val localDef = LocalDef( RefinedType(AnyType, isExact = false, isNullable = false), mutable = false, @@ -1160,7 +1160,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { case PreTransLocalDef(localDef @ LocalDef(tpe, _, replacement)) => replacement match { case ReplaceWithRecordVarRef(name, recordType, used, cancelFun) => - used.value = true + used.value = Used PreTransRecordTree( VarRef(LocalIdent(name))(recordType), tpe, cancelFun) @@ -1369,7 +1369,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { (name, used) } - if (used.value) { + if (used.value.isUsed) { val ident = LocalIdent(name) val varDef = resolveLocalDef(value) match { case PreTransRecordTree(valueTree, valueTpe, cancelFun) => @@ -1792,8 +1792,8 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { closure @ TentativeClosureReplacement( captureParams, params, body, captureLocalDefs, alreadyUsed, cancelFun))) - if !alreadyUsed.value && argsNoSpread.size <= params.size => - alreadyUsed.value = true + if !alreadyUsed.value.isUsed && argsNoSpread.size <= params.size => + alreadyUsed.value = Used val missingArgCount = params.size - argsNoSpread.size val expandedArgs = if (missingArgCount == 0) argsNoSpread @@ -4014,7 +4014,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { } yield { val (newName, newOriginalName) = freshLocalName(name, originalName, mutable) val localDef = LocalDef(RefinedType(ptpe), mutable, - ReplaceWithVarRef(newName, newSimpleState(true), None)) + ReplaceWithVarRef(newName, newSimpleState(Used), None)) val newParamDef = ParamDef(LocalIdent(newName)(ident.pos), newOriginalName, ptpe, mutable)(p.pos) ((name -> localDef), newParamDef) @@ -4271,7 +4271,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { // Otherwise, we effectively declare a new binding val (newName, newOriginalName) = freshLocalName(bindingName, mutable) - val used = newSimpleState(false) + val used = newSimpleState[IsUsed](Unused) val (replacement, refinedType) = resolveRecordType(value) match { case Some((recordType, cancelFun)) => @@ -4374,7 +4374,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { case PreTransTree(VarRef(LocalIdent(refName)), _) if !localIsMutable(refName) => buildInner(LocalDef(computeRefinedType(), false, - ReplaceWithVarRef(refName, newSimpleState(true), None)), cont) + ReplaceWithVarRef(refName, newSimpleState(Used), None)), cont) case _ => withDedicatedVar(computeRefinedType()) @@ -4667,7 +4667,7 @@ private[optimizer] object OptimizerCore { implicit pos: Position): Tree = replacement match { case ReplaceWithVarRef(name, used, _) => - used.value = true + used.value = Used VarRef(LocalIdent(name))(tpe.base) /* Allocate an instance of RuntimeLong on the fly. @@ -4676,7 +4676,7 @@ private[optimizer] object OptimizerCore { */ case ReplaceWithRecordVarRef(name, recordType, used, _) if tpe.base == ClassType(LongImpl.RuntimeLongClass) => - used.value = true + used.value = Used createNewLong(VarRef(LocalIdent(name))(recordType)) case ReplaceWithRecordVarRef(_, _, _, cancelFun) => @@ -4738,12 +4738,12 @@ private[optimizer] object OptimizerCore { private sealed abstract class LocalDefReplacement private final case class ReplaceWithVarRef(name: LocalName, - used: SimpleState[Boolean], + used: SimpleState[IsUsed], longOpTree: Option[() => Tree]) extends LocalDefReplacement private final case class ReplaceWithRecordVarRef(name: LocalName, recordType: RecordType, - used: SimpleState[Boolean], + used: SimpleState[IsUsed], cancelFun: CancelFun) extends LocalDefReplacement private final case class ReplaceWithThis() extends LocalDefReplacement @@ -4763,7 +4763,7 @@ private[optimizer] object OptimizerCore { private final case class TentativeClosureReplacement( captureParams: List[ParamDef], params: List[ParamDef], body: Tree, captureValues: List[LocalDef], - alreadyUsed: SimpleState[Boolean], + alreadyUsed: SimpleState[IsUsed], cancelFun: CancelFun) extends LocalDefReplacement private final case class InlineClassBeingConstructedReplacement( @@ -4898,8 +4898,8 @@ private[optimizer] object OptimizerCore { localDef.replacement) def isAlreadyUsed: Boolean = (localDef.replacement: @unchecked) match { - case ReplaceWithVarRef(_, used, _) => used.value - case ReplaceWithRecordVarRef(_, _, used, _) => used.value + case ReplaceWithVarRef(_, used, _) => used.value.isUsed + case ReplaceWithRecordVarRef(_, _, used, _) => used.value.isUsed } } @@ -5670,4 +5670,14 @@ private[optimizer] object OptimizerCore { new FieldID(ownerClassName, fieldDef.name.name) } + private sealed abstract class IsUsed { + def isUsed: Boolean + } + private case object Used extends IsUsed { + override def isUsed: Boolean = true + } + private case object Unused extends IsUsed { + override def isUsed: Boolean = false + } + } From d589ff8b177661513fdf3f501e374df3441b5174 Mon Sep 17 00:00:00 2001 From: David Barri Date: Fri, 11 Mar 2022 12:57:09 +1100 Subject: [PATCH 11/20] Fix #4646: Optimize away unnecessary closure capture-params --- .../frontend/optimizer/OptimizerCore.scala | 74 ++++++++++++------ .../org/scalajs/linker/OptimizerTest.scala | 76 +++++++++++++++++++ .../linker/testutils/TestIRBuilder.scala | 11 +-- project/Build.scala | 4 +- 4 files changed, 133 insertions(+), 32 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index 04137d4703..4e2cec4ae3 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -135,7 +135,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { throw new AssertionError("Methods to optimize must be concrete") } - val (newParams, newBody1) = try { + val (newParamsWithUsage, newBody1) = try { transformIsolatedBody(Some(myself), thisType, params, resultType, body, Set.empty) } catch { @@ -151,6 +151,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { val newBody = if (originalDef.methodName == NoArgConstructorName) tryElimStoreModule(newBody1) else newBody1 + val newParams = newParamsWithUsage.map(_._1) MethodDef(static, name, originalName, newParams, resultType, Some(newBody))(originalDef.optimizerHints, None)(originalDef.pos) } catch { @@ -647,8 +648,12 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { } case Closure(arrow, captureParams, params, restParam, body, captureValues) => - transformClosureCommon(arrow, captureParams, params, restParam, body, - captureValues.map(transformExpr)) + trampoline { + pretransformExprs(captureValues) { tcaptureValues => + transformClosureCommon(arrow, captureParams, params, restParam, body, + tcaptureValues)(finishTransform(isStat)) + } + } case CreateJSClass(className, captureValues) => CreateJSClass(className, captureValues.map(transformExpr)) @@ -673,19 +678,39 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { private def transformClosureCommon(arrow: Boolean, captureParams: List[ParamDef], params: List[ParamDef], restParam: Option[ParamDef], body: Tree, - newCaptureValues: List[Tree])( - implicit scope: Scope, pos: Position): Closure = { + tcaptureValues: List[PreTransform])(cont: PreTransCont)( + implicit scope: Scope, pos: Position): TailRec[Tree] = { val thisType = if (arrow) NoType else AnyType - val (allNewParams, newBody) = transformIsolatedBody(None, thisType, + + val (allNewParamsWithUse, newBody) = transformIsolatedBody(None, thisType, captureParams ++ params ++ restParam, AnyType, body, scope.implsBeingInlined) - val (newCaptureParams, newParams, newRestParam) = { - val (c, t) = allNewParams.splitAt(captureParams.size) - if (restParam.isDefined) (c, t.init, Some(t.last)) - else (c, t, None) + + val (newCaptureParamsWithUse, newParamsWithUse) = allNewParamsWithUse.splitAt(captureParams.size) + + val (newParams, newRestParam) = { + val t = newParamsWithUse.map(_._1).toList + if (restParam.isDefined) (t.init, Some(t.last)) + else (t, None) } - Closure(arrow, newCaptureParams, newParams, newRestParam, newBody, newCaptureValues) + val captureBindings = { + newCaptureParamsWithUse.iterator.zip(tcaptureValues.iterator).map { case ((param, _), value) => + Binding.temp(param.name.name, param.ptpe, mutable = false, value) + }.toList + } + + withNewLocalDefs(captureBindings) { (localDefs, cont1) => + val (finalCaptureParams, finalCaptureValues) = (for { + (localDef, (param, use)) <- localDefs.iterator.zip(newCaptureParamsWithUse.iterator) + if use.isUsed + } yield { + param -> localDef.newReplacement + }).toList.unzip + val newClosure = Closure(arrow, finalCaptureParams, newParams, newRestParam, newBody, finalCaptureValues) + val newTree = PreTransTree(newClosure, RefinedType(AnyType, isExact = false, isNullable = false)) + cont1(newTree) + } (cont) } private def transformBlock(tree: Block, isStat: Boolean)( @@ -892,11 +917,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { case Closure(arrow, captureParams, params, restParam, body, captureValues) => pretransformExprs(captureValues) { tcaptureValues => def default(): TailRec[Tree] = { - val newClosure = transformClosureCommon(arrow, captureParams, - params, restParam, body, tcaptureValues.map(finishTransformExpr)) - cont(PreTransTree( - newClosure, - RefinedType(AnyType, isExact = false, isNullable = false))) + transformClosureCommon(arrow, captureParams, params, restParam, body, tcaptureValues)(cont) } if (!arrow || restParam.isDefined) { @@ -4005,19 +4026,20 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { } } - def transformIsolatedBody(optTarget: Option[MethodID], + private def transformIsolatedBody(optTarget: Option[MethodID], thisType: Type, params: List[ParamDef], resultType: Type, body: Tree, - alreadyInlining: Set[Scope.InliningID]): (List[ParamDef], Tree) = { - val (paramLocalDefs, newParamDefs) = (for { + alreadyInlining: Set[Scope.InliningID]): (List[(ParamDef, IsUsed)], Tree) = { + + val (paramLocalDefs, newParamDefsAndRepls) = (for { p @ ParamDef(ident @ LocalIdent(name), originalName, ptpe, mutable) <- params } yield { val (newName, newOriginalName) = freshLocalName(name, originalName, mutable) - val localDef = LocalDef(RefinedType(ptpe), mutable, - ReplaceWithVarRef(newName, newSimpleState(Used), None)) - val newParamDef = ParamDef(LocalIdent(newName)(ident.pos), - newOriginalName, ptpe, mutable)(p.pos) - ((name -> localDef), newParamDef) + val replacement = ReplaceWithVarRef(newName, newSimpleState(Unused), None) + val localDef = LocalDef(RefinedType(ptpe), mutable, replacement) + val localIdent = LocalIdent(newName)(ident.pos) + val newParamDef = ParamDef(localIdent, newOriginalName, ptpe, mutable)(p.pos) + (name -> localDef, newParamDef -> replacement) }).unzip val thisLocalDef = @@ -4043,7 +4065,9 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { val scope = Scope.Empty.inlining(inlining).withEnv(env) val newBody = transform(body, resultType == NoType)(scope) - (newParamDefs, newBody) + val newParamDefsWithUsage = newParamDefsAndRepls.map(t => (t._1, t._2.used.value)) + + (newParamDefsWithUsage, newBody) } private def pretransformLabeled(oldLabelName: LabelName, resultType: Type, diff --git a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala index 129378b7d8..00137ae1f3 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala @@ -20,6 +20,7 @@ import org.junit.Assert._ import org.scalajs.ir.ClassKind import org.scalajs.ir.EntryPointsInfo import org.scalajs.ir.Names._ +import org.scalajs.ir.Traversers.Traverser import org.scalajs.ir.Trees._ import org.scalajs.ir.Trees.MemberNamespace._ import org.scalajs.ir.Types._ @@ -288,6 +289,81 @@ class OptimizerTest { testLink(classDefs, MainTestModuleInitializers) } + @Test + def testCaptureElimination(): AsyncResult = await { + val sideEffect = m("sideEffect", List(I), I) + + val x = LocalName("x") + val x2 = LocalName("x2") + val x4 = LocalName("x4") + + val classDefs = Seq( + classDef( + MainTestClassName, + kind = ClassKind.Class, + superClass = Some(ObjectClass), + memberDefs = List( + trivialCtor(MainTestClassName), + // @noinline static def sideEffect(x: Int): Int = x + MethodDef(EMF.withNamespace(PublicStatic), sideEffect, NON, + List(paramDef(x, IntType)), IntType, Some(VarRef(x)(IntType)))( + EOH.withNoinline(true), None), + /* static def main(args: String[]) { + * console.log(arrow-lambda< + * x1 = sideEffect(1), + * x2 = sideEffect(2), + * ... + * x5 = sideEffect(5), + * >() = { + * console.log(x4); + * console.log(x2); + * }); + * } + */ + mainMethodDef({ + val closure = Closure( + arrow = true, + (1 to 5).toList.map(i => paramDef(LocalName("x" + i), IntType)), + Nil, + None, + Block( + consoleLog(VarRef(x4)(IntType)), + consoleLog(VarRef(x2)(IntType)) + ), + (1 to 5).toList.map(i => ApplyStatic(EAF, MainTestClassName, sideEffect, List(int(i)))(IntType)) + ) + consoleLog(closure) + }) + ) + ) + ) + + for (moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers)) yield { + val mainClassDef = moduleSet.modules.flatMap(_.classDefs) + .find(_.className == MainTestClassName).get + val mainMethodDef = mainClassDef.methods.map(_.value) + .find(_.name.name == MainMethodName).get + + var lastSideEffectFound = 0 + var closureParams = List.empty[List[LocalName]] + new Traverser { + override def traverse(tree: Tree): Unit = { + super.traverse(tree) + tree match { + case ApplyStatic(_, MainTestClassName, MethodIdent(`sideEffect`), List(IntLiteral(i))) => + assertEquals("wrong side effect ordering", lastSideEffectFound + 1, i) + lastSideEffectFound = i + case c: Closure => + closureParams :+= c.captureParams.map(_.name.name) + case _ => + () + } + } + }.traverseMemberDef(mainMethodDef) + assertEquals("wrong number of side effect calls", 5, lastSideEffectFound) + assertEquals("wrong closure params", List(List(x2, x4)), closureParams) + } + } } object OptimizerTest { diff --git a/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala b/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala index 188c4db210..9f30a6ab26 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala @@ -38,6 +38,7 @@ object TestIRBuilder { val Z = BooleanRef val O = ClassRef(ObjectClass) val T = ClassRef(BoxedStringClass) + val AT = ArrayTypeRef(ClassRef(BoxedStringClass), 1) def m(name: String, paramTypeRefs: List[TypeRef], resultTypeRef: TypeRef): MethodName = MethodName(name, paramTypeRefs, resultTypeRef) @@ -85,13 +86,13 @@ object TestIRBuilder { EOH, None) } + val MainMethodName: MethodName = m("main", List(AT), VoidRef) + def mainMethodDef(body: Tree): MethodDef = { - val stringArrayTypeRef = ArrayTypeRef(ClassRef(BoxedStringClass), 1) - val stringArrayType = ArrayType(stringArrayTypeRef) - val argsParamDef = paramDef("args", stringArrayType) + val argsParamDef = paramDef("args", ArrayType(AT)) MethodDef(MemberFlags.empty.withNamespace(MemberNamespace.PublicStatic), - m("main", List(stringArrayTypeRef), VoidRef), NON, - List(argsParamDef), NoType, Some(body))(EOH, None) + MainMethodName, NON, List(argsParamDef), NoType, Some(body))( + EOH, None) } def consoleLog(expr: Tree): Tree = diff --git a/project/Build.scala b/project/Build.scala index b6d1fe03c6..976657387a 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1735,7 +1735,7 @@ object Build { case Default2_12ScalaVersion => Some(ExpectedSizes( - fastLink = 784000 to 785000, + fastLink = 783000 to 784000, fullLink = 150000 to 151000, fastLinkGz = 92000 to 93000, fullLinkGz = 37000 to 38000, @@ -1744,7 +1744,7 @@ object Build { case Default2_13ScalaVersion => Some(ExpectedSizes( fastLink = 734000 to 735000, - fullLink = 158000 to 159000, + fullLink = 157000 to 158000, fastLinkGz = 92000 to 93000, fullLinkGz = 40000 to 41000, )) From fcc323b3d112cc9a0621529a3febf51569c42658 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 6 Mar 2022 15:49:27 +0100 Subject: [PATCH 12/20] Replace gitter link with link to Discord --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d5907474dd..add1859b89 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@

-[![Join the chat at https://gitter.im/scala-js/scala-js](https://badges.gitter.im/scala-js/scala-js.svg)](https://gitter.im/scala-js/scala-js) +Chat: [#scala-js](https://discord.com/invite/scala) on Discord. This is the repository for [Scala.js, the Scala to JavaScript compiler](https://www.scala-js.org/). From df0283f001d39eea64ddf107d009cd2bc4702542 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Fri, 18 Mar 2022 16:47:15 +0100 Subject: [PATCH 13/20] Fix #4642: Specify `linker.*` to be stable --- VERSIONING.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/VERSIONING.md b/VERSIONING.md index e51819928d..941d33977f 100644 --- a/VERSIONING.md +++ b/VERSIONING.md @@ -17,7 +17,8 @@ Severe changes can break the ecosystem of sbt plugins and other build tools, but not the ecosystem of libraries (which would be major). Severe changes should be done only if absolutely necessary. The following are considered severe changes: -* Backward binary incompatible changes in `linker.interface.*` or `sbtplugin.*` +* Backward binary incompatible changes in `linker.*` or `linker.interface.*` +* Backward binary incompatible changes in `sbtplugin.*` * Backward binary incompatible changes in `testadapter.*` Severe changes are difficult from a versioning point of view, since they require @@ -42,19 +43,19 @@ The following changes must cause a minor version bump. * Forward incompatible change in the IR * Backward source incompatible change at the language level or at the standard library level (including any addition of public API in the stdlib) -* Backward source incompatible change in `linker.interface.*` or `sbtplugin.*` - (including any addition of public API) +* Backward source incompatible change in `linker.*`, `linker.interface.*` + or `sbtplugin.*` (including any addition of public API) * Backward source incompatible changes in `testadapter.*` -* Backward binary incompatible changes in `ir.*`, `linker.interface.unstable.*`, - `linker.*` or `linker.standard.*` +* Backward binary incompatible changes in `ir.*`, `linker.interface.unstable.*` + or `linker.standard.*` # Patch Changes All other changes cause a patch version bump only. Explicitly (but not exhaustively): -* Backward source incompatible change in `ir.*`, `linker.interface.unstable.*`, - `linker.*` or `linker.standard.*` +* Backward source incompatible change in `ir.*`, `linker.interface.unstable.*` + or `linker.standard.*` * Backward source/binary incompatible changes elsewhere in `linker.**` * Fixes or additions in the JDK libs (since they are always backward source and binary compatible) From fa454de86b9c4a803de7314b21a1d4a2a26beebc Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 30 Jan 2022 13:51:51 +0100 Subject: [PATCH 14/20] Simplify method attribute handling Instead of providing forwarders for all the fields, we simply provide access to the entire attribute structure. --- .../frontend/optimizer/IncOptimizer.scala | 2 +- .../frontend/optimizer/OptimizerCore.scala | 51 +++++++++---------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala index 9cc7db4f1e..280cf00f2d 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala @@ -846,7 +846,7 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: var originalDef: MethodDef = _ var optimizedMethodDef: Versioned[MethodDef] = _ - var attributes: Attributes = _ + var attributes: OptimizerCore.MethodAttributes = _ def enclosingClassName: ClassName = owner.className diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index 4e2cec4ae3..86a755debd 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -1546,8 +1546,8 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { if (intrinsicCode >= 0) { callIntrinsic(intrinsicCode, flags, Some(treceiver), methodName, targs, isStat, usePreTransform)(cont) - } else if (target.inlineable && ( - target.shouldInline || + } else if (target.attributes.inlineable && ( + target.attributes.shouldInline || shouldInlineBecauseOfArgs(target, treceiver :: targs))) { /* When inlining a single method, the declared type of the `this` * value is its enclosing class. @@ -1580,7 +1580,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { private def canMultiInline(impls: List[MethodID]): Boolean = { // TODO? Inline multiple non-forwarders with the exact same body? - impls.forall(impl => impl.isForwarder && impl.inlineable) && + impls.forall(impl => impl.attributes.isForwarder && impl.attributes.inlineable) && (getMethodBody(impls.head).body.get match { // Trait impl forwarder case ApplyStatic(flags, staticCls, MethodIdent(methodName), _) => @@ -1667,8 +1667,8 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { callIntrinsic(intrinsicCode, flags, Some(treceiver), methodName, targs, isStat, usePreTransform)(cont) } else { - val shouldInline = target.inlineable && ( - target.shouldInline || + val shouldInline = target.attributes.inlineable && ( + target.attributes.shouldInline || shouldInlineBecauseOfArgs(target, treceiver :: targs)) val allocationSites = (treceiver :: targs).map(_.tpe.allocationSite) @@ -1710,8 +1710,8 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { callIntrinsic(intrinsicCode, flags, None, methodName, targs, isStat, usePreTransform)(cont) } else { - val shouldInline = target.inlineable && ( - target.shouldInline || shouldInlineBecauseOfArgs(target, targs)) + val shouldInline = target.attributes.inlineable && ( + target.attributes.shouldInline || shouldInlineBecauseOfArgs(target, targs)) val allocationSites = targs.map(_.tpe.allocationSite) val beingInlined = scope.implsBeingInlined((allocationSites, target)) @@ -1960,7 +1960,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { cont: PreTransCont)( implicit scope: Scope, pos: Position): TailRec[Tree] = { - require(target.inlineable) + require(target.attributes.inlineable) attemptedInlining += target @@ -5376,9 +5376,7 @@ private[optimizer] object OptimizerCore { trait AbstractMethodID { def enclosingClassName: ClassName def methodName: MethodName - def inlineable: Boolean - def shouldInline: Boolean - def isForwarder: Boolean + def attributes: MethodAttributes final def is(className: ClassName, methodName: MethodName): Boolean = this.enclosingClassName == className && this.methodName == methodName @@ -5391,15 +5389,7 @@ private[optimizer] object OptimizerCore { def originalDef: MethodDef def thisType: Type - protected type Attributes = MethodImpl.Attributes - - protected def attributes: Attributes - - final def inlineable: Boolean = attributes.inlineable - final def shouldInline: Boolean = attributes.shouldInline - final def isForwarder: Boolean = attributes.isForwarder - - protected def computeNewAttributes(): Attributes = { + protected def computeNewAttributes(): MethodAttributes = { val MethodDef(_, MethodIdent(methodName), _, params, _, optBody) = originalDef val body = optBody getOrElse { throw new AssertionError("Methods in optimizer must be concrete") @@ -5468,17 +5458,22 @@ private[optimizer] object OptimizerCore { } } - MethodImpl.Attributes(inlineable, shouldInline, isForwarder) + new MethodAttributes(inlineable, shouldInline, isForwarder) } } - object MethodImpl { - final case class Attributes( - inlineable: Boolean, - shouldInline: Boolean, - isForwarder: Boolean - ) - } + /* This is a "broken" case class so we get equals (and hashCode) for free. + * + * This hack is somewhat acceptable, because: + * - it is only part of the OptimizerCore / IncOptimizer interface. + * - the risk of getting equals wrong is high: it only affects the incremental + * behavior of the optimizer, which we have few tests for. + */ + final case class MethodAttributes private[OptimizerCore] ( + private[OptimizerCore] val inlineable: Boolean, + private[OptimizerCore] val shouldInline: Boolean, + private[OptimizerCore] val isForwarder: Boolean + ) private object MaybeUnbox { def unapply(tree: Tree): Some[(Tree, Any)] = tree match { From e3c38dfe879892b753c96bb679d76885402ece93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 29 Mar 2022 15:28:50 +0200 Subject: [PATCH 15/20] Bump to 1.10.0-SNAPSHOT for the upcoming changes to UUID. --- ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala b/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala index ade4a9b81d..512f04c890 100644 --- a/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala +++ b/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala @@ -17,7 +17,7 @@ import java.util.concurrent.ConcurrentHashMap import scala.util.matching.Regex object ScalaJSVersions extends VersionChecks( - current = "1.9.1-SNAPSHOT", + current = "1.10.0-SNAPSHOT", binaryEmitted = "1.8" ) From 7d8d34e60395b3fc44ecc5ce22b13df7aa191ead Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 30 Jan 2022 13:51:51 +0100 Subject: [PATCH 16/20] Fix #4315: Optimize dynamicImports of native imports --- .../frontend/optimizer/IncOptimizer.scala | 74 +++++ .../frontend/optimizer/OptimizerCore.scala | 289 ++++++++++++++++-- .../org/scalajs/linker/OptimizerTest.scala | 80 ++++- .../linker/testutils/TestIRBuilder.scala | 22 +- .../jsinterop/SJSDynamicImportTest.scala | 65 ++++ 5 files changed, 478 insertions(+), 52 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala index 280cf00f2d..f237114c67 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala @@ -28,6 +28,7 @@ import org.scalajs.logging._ import org.scalajs.linker._ import org.scalajs.linker.backend.emitter.LongImpl import org.scalajs.linker.frontend.LinkingUnit +import org.scalajs.linker.interface.ModuleKind import org.scalajs.linker.standard._ import org.scalajs.linker.CollectionsCompat.MutableMapCompatOps @@ -709,6 +710,8 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: private val staticCallers = mutable.ArrayBuffer.fill[MethodCallers](MemberNamespace.Count)(collOps.emptyMap) + private val jsNativeImportsAskers = collOps.emptyMap[MethodImpl, Unit] + private var _ancestors: List[ClassName] = linkedClass.ancestors private val _instantiatedSubclasses = collOps.emptyMap[Class, Unit] @@ -719,6 +722,17 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: } } + /* For now, we track all JS native imports together (the class itself and native members). + * + * This is more to avoid unnecessary tracking than due to an intrinsic reason. + */ + + private type JSNativeImports = + (Option[JSNativeLoadSpec.Import], Map[MethodName, JSNativeLoadSpec.Import]) + + private var jsNativeImports: JSNativeImports = + computeJSNativeImports(linkedClass) + override def toString(): String = s"intf ${className.nameString}" @@ -767,6 +781,21 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: _ancestors } + /** PROCESS PASS ONLY. Concurrency safe except with [[updateWith]]. */ + def askJSNativeImport(asker: MethodImpl): Option[JSNativeLoadSpec.Import] = { + jsNativeImportsAskers.put(asker, ()) + asker.registerTo(this) + jsNativeImports._1 + } + + /** PROCESS PASS ONLY. Concurrency safe except with [[updateWith]]. */ + def askJSNativeImport(methodName: MethodName, + asker: MethodImpl): Option[JSNativeLoadSpec.Import] = { + jsNativeImportsAskers.put(asker, ()) + asker.registerTo(this) + jsNativeImports._2.get(methodName) + } + @inline def staticLike(namespace: MemberNamespace): StaticLikeNamespace = staticLikes(namespace.ordinal) @@ -780,6 +809,14 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: ancestorsAskers.clear() } + // Update jsNativeImports + val newJSNativeImports = computeJSNativeImports(linkedClass) + if (jsNativeImports != newJSNativeImports) { + jsNativeImports = newJSNativeImports + jsNativeImportsAskers.keysIterator.foreach(_.tag()) + jsNativeImportsAskers.clear() + } + // Update static likes for (staticLike <- staticLikes) { val (_, changed, _) = staticLike.updateWith(linkedClass) @@ -817,6 +854,31 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: ancestorsAskers.remove(dependee) dynamicCallers.valuesIterator.foreach(_.remove(dependee)) staticCallers.foreach(_.valuesIterator.foreach(_.remove(dependee))) + jsNativeImportsAskers.remove(dependee) + } + + private def computeJSNativeImports(linkedClass: LinkedClass): JSNativeImports = { + def maybeImport(spec: JSNativeLoadSpec): Option[JSNativeLoadSpec.Import] = spec match { + case i: JSNativeLoadSpec.Import => + Some(i) + + case JSNativeLoadSpec.ImportWithGlobalFallback(i, _) => + if (config.coreSpec.moduleKind != ModuleKind.NoModule) Some(i) + else None + + case _: JSNativeLoadSpec.Global => + None + } + + val clazz = linkedClass.jsNativeLoadSpec.flatMap(maybeImport(_)) + val nativeMembers = for { + member <- linkedClass.jsNativeMembers + jsImport <- maybeImport(member.jsNativeLoadSpec) + } yield { + member.name.name -> jsImport + } + + (clazz, nativeMembers.toMap) } } @@ -960,6 +1022,8 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: /** All methods are PROCESS PASS ONLY */ private class Optimizer extends OptimizerCore(config) { + import OptimizerCore.ImportTarget + type MethodID = MethodImpl val myself: MethodImpl.this.type = MethodImpl.this @@ -989,6 +1053,16 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps: className: ClassName): Option[OptimizerCore.InlineableClassStructure] = { classes(className).tryNewInlineable } + + protected def getJSNativeImportOf( + target: ImportTarget): Option[JSNativeLoadSpec.Import] = { + target match { + case ImportTarget.Class(className) => + getInterface(className).askJSNativeImport(myself) + case ImportTarget.Member(className, methodName) => + getInterface(className).askJSNativeImport(methodName, myself) + } + } } } diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index 86a755debd..aa5f88bcc7 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -79,6 +79,13 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { protected def tryNewInlineableClass( className: ClassName): Option[InlineableClassStructure] + /** Returns the jsNativeLoadSpec of the given import target if it is an Import. + * + * Otherwise returns None. + */ + protected def getJSNativeImportOf( + target: ImportTarget): Option[JSNativeLoadSpec.Import] + private val localNameAllocator = new FreshNameAllocator.Local /** An allocated local variable name is mutable iff it belongs to this set. */ @@ -135,9 +142,10 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { throw new AssertionError("Methods to optimize must be concrete") } + implicit val scope = Scope.Empty + val (newParamsWithUsage, newBody1) = try { - transformIsolatedBody(Some(myself), thisType, params, resultType, body, - Set.empty) + transformIsolatedBody(Some(myself), thisType, params, resultType, body) } catch { case _: TooManyRollbacksException => localNameAllocator.clear() @@ -145,8 +153,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { labelNameAllocator.clear() stateBackupChain = Nil disableOptimisticOptimizations = true - transformIsolatedBody(Some(myself), thisType, params, resultType, - body, Set.empty) + transformIsolatedBody(Some(myself), thisType, params, resultType, body) } val newBody = if (originalDef.methodName == NoArgConstructorName) tryElimStoreModule(newBody1) @@ -505,8 +512,10 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { finishTransform(isStat)) } - case ApplyDynamicImport(flags, className, method, args) => - ApplyDynamicImport(flags, className, method, args.map(transformExpr(_))) + case tree: ApplyDynamicImport => + trampoline { + pretransformApplyDynamicImport(tree, isStat)(finishTransform(isStat)) + } case tree: UnaryOp => trampoline { @@ -658,11 +667,19 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { case CreateJSClass(className, captureValues) => CreateJSClass(className, captureValues.map(transformExpr)) + case SelectJSNativeMember(className, MethodIdent(member)) => + transformJSLoadCommon(ImportTarget.Member(className, member), tree) + + case LoadJSModule(className) => + transformJSLoadCommon(ImportTarget.Class(className), tree) + + case LoadJSConstructor(className) => + transformJSLoadCommon(ImportTarget.Class(className), tree) + // Trees that need not be transformed case _:Skip | _:Debugger | _:LoadModule | _:SelectStatic | - _:SelectJSNativeMember | _:JSNewTarget | _:JSImportMeta | - _:LoadJSConstructor | _:LoadJSModule | _:JSLinkingInfo | + _:JSNewTarget | _:JSImportMeta | _:JSLinkingInfo | _:JSGlobalRef | _:JSTypeOfGlobalRef | _:Literal => tree @@ -684,7 +701,7 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { val thisType = if (arrow) NoType else AnyType val (allNewParamsWithUse, newBody) = transformIsolatedBody(None, thisType, - captureParams ++ params ++ restParam, AnyType, body, scope.implsBeingInlined) + captureParams ++ params ++ restParam, AnyType, body) val (newCaptureParamsWithUse, newParamsWithUse) = allNewParamsWithUse.splitAt(captureParams.size) @@ -694,22 +711,32 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { else (t, None) } - val captureBindings = { + withCaptures(newCaptureParamsWithUse, tcaptureValues) { (finalCaptureParams, finalCaptureValues, cont1) => + val newClosure = Closure(arrow, finalCaptureParams, newParams, newRestParam, newBody, finalCaptureValues) + val newTree = PreTransTree(newClosure, RefinedType(AnyType, isExact = false, isNullable = false)) + cont1(newTree) + } (cont) + } + + private def withCaptures(newCaptureParamsWithUse: List[(ParamDef, IsUsed)], + tcaptureValues: List[PreTransform])( + buildInner: (List[ParamDef], List[Tree], PreTransCont) => TailRec[Tree])( + cont: PreTransCont)(implicit scope: Scope, pos: Position) = { + val bindings = { newCaptureParamsWithUse.iterator.zip(tcaptureValues.iterator).map { case ((param, _), value) => Binding.temp(param.name.name, param.ptpe, mutable = false, value) }.toList } - withNewLocalDefs(captureBindings) { (localDefs, cont1) => + withNewLocalDefs(bindings) { (localDefs, cont1) => val (finalCaptureParams, finalCaptureValues) = (for { (localDef, (param, use)) <- localDefs.iterator.zip(newCaptureParamsWithUse.iterator) if use.isUsed } yield { param -> localDef.newReplacement }).toList.unzip - val newClosure = Closure(arrow, finalCaptureParams, newParams, newRestParam, newBody, finalCaptureValues) - val newTree = PreTransTree(newClosure, RefinedType(AnyType, isExact = false, isNullable = false)) - cont1(newTree) + + buildInner(finalCaptureParams, finalCaptureValues, cont1) } (cont) } @@ -859,6 +886,9 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { pretransformApplyStatic(tree, isStat = false, usePreTransform = true)(cont) + case tree: ApplyDynamicImport => + pretransformApplyDynamicImport(tree, isStat = false)(cont) + case tree: UnaryOp => pretransformUnaryOp(tree)(cont) @@ -1726,6 +1756,78 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { } } + private def pretransformApplyDynamicImport(tree: ApplyDynamicImport, isStat: Boolean)( + cont: PreTransCont)( + implicit outerScope: Scope): TailRec[Tree] = { + + val ApplyDynamicImport(flags, className, method, args) = tree + implicit val pos = tree.pos + + def treeNotInlined0(transformedArgs: List[Tree]) = + cont(PreTransTree(ApplyDynamicImport(flags, className, method, transformedArgs), + RefinedType(AnyType))) + + def treeNotInlined = treeNotInlined0(args.map(transformExpr)) + + val targetMethod = + staticCall(className, MemberNamespace.forStaticCall(flags), method.name) + + if (!targetMethod.attributes.inlineable) { + treeNotInlined + } else { + val maybeImportTarget = targetMethod.attributes.jsDynImportInlineTarget.orElse { + targetMethod.attributes.jsDynImportThunkFor.flatMap { thunkTarget => + val id = staticCall(className, MemberNamespace.Public, thunkTarget) + if (id.attributes.inlineable) + id.attributes.jsDynImportInlineTarget + else + None + } + } + + val maybeInlined = for { + importTarget <- maybeImportTarget + jsNativeLoadSpec <- getJSNativeImportOf(importTarget) + } yield { + pretransformExprs(args) { targs => + tryOrRollback { cancelFun => + val moduleName = freshLocalNameWithoutOriginalName( + LocalName("module"), mutable = false) + + val moduleParam = + ParamDef(LocalIdent(moduleName), NoOriginalName, AnyType, mutable = false) + + val importReplacement = ImportReplacement(importTarget, moduleName, + jsNativeLoadSpec.path, newSimpleState(Unused), cancelFun) + + val (newCaptureParamsWithUse, newBody) = { + val newScope = outerScope.withImportReplacement(importReplacement) + val MethodDef(_, _, _, params, resultType, Some(body)) = getMethodBody(targetMethod) + transformIsolatedBody(Some(targetMethod), NoType, params, resultType, body)(newScope) + } + + if (!importReplacement.used.value.isUsed) + cancelFun() + + withCaptures(newCaptureParamsWithUse, targs) { (captureParams, captureValues, cont1) => + val inlinedClosure = Closure(arrow = true, captureParams, List(moduleParam), + restParam = None, newBody, captureValues) + + val newTree = JSImport(config.coreSpec.moduleKind, jsNativeLoadSpec.module, + inlinedClosure) + + cont1(PreTransTree(newTree)) + } (cont) + } { () => + treeNotInlined0(targs.map(finishTransformExpr)) + } + } + } + + maybeInlined.getOrElse(treeNotInlined) + } + } + private def pretransformJSSelect(tree: JSSelect, isLhsOfAssign: Boolean)( cont: PreTransCont)( implicit scope: Scope): TailRec[Tree] = { @@ -1794,6 +1896,24 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { } } + private def transformJSLoadCommon(target: ImportTarget, tree: Tree)( + implicit scope: Scope, pos: Position): Tree = { + scope.importReplacement match { + case Some(ImportReplacement(expectedTarget, moduleVarName, path, used, cancelFun)) => + if (target != expectedTarget) + cancelFun() + + used.value = Used + val module = VarRef(LocalIdent(moduleVarName))(AnyType) + path.foldLeft[Tree](module) { (inner, pathElem) => + JSSelect(inner, StringLiteral(pathElem)) + } + + case None => + tree + } + } + private def pretransformJSFunctionApply(tree: JSFunctionApply, isStat: Boolean, usePreTransform: Boolean)( cont: PreTransCont)( @@ -4027,9 +4147,8 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { } private def transformIsolatedBody(optTarget: Option[MethodID], - thisType: Type, params: List[ParamDef], resultType: Type, - body: Tree, - alreadyInlining: Set[Scope.InliningID]): (List[(ParamDef, IsUsed)], Tree) = { + thisType: Type, params: List[ParamDef], resultType: Type, body: Tree)( + implicit outerScope: Scope): (List[(ParamDef, IsUsed)], Tree) = { val (paramLocalDefs, newParamDefsAndRepls) = (for { p @ ParamDef(ident @ LocalIdent(name), originalName, ptpe, mutable) <- params @@ -4050,19 +4169,21 @@ private[optimizer] abstract class OptimizerCore(config: CommonPhaseConfig) { false, ReplaceWithThis())) } - val inlining = optTarget.fold(alreadyInlining) { target => + val inlining = optTarget.map { target => val allocationSiteCount = paramLocalDefs.size + (if (thisLocalDef.isDefined) 1 else 0) val allocationSites = List.fill(allocationSiteCount)(AllocationSite.Anonymous) - alreadyInlining + ((allocationSites, target)) - } + allocationSites -> target + }.toSet[Scope.InliningID] + val env = { val envWithThis = thisLocalDef.fold(OptEnv.Empty)(OptEnv.Empty.withThisLocalDef(_)) envWithThis.withLocalDefs(paramLocalDefs) } - val scope = Scope.Empty.inlining(inlining).withEnv(env) + val scope = outerScope.inlining(inlining).withEnv(env) + val newBody = transform(body, resultType == NoType)(scope) val newParamDefsWithUsage = newParamDefsAndRepls.map(t => (t._1, t._2.used.value)) @@ -4804,6 +4925,14 @@ private[optimizer] object OptimizerCore { elemLocalDefs: Vector[LocalDef], cancelFun: CancelFun) extends LocalDefReplacement + /** Replaces an import target. Part of the ApplyDynamicImport inlining. + * + * @note This is **not** a LocalDefReplacement. + */ + private final case class ImportReplacement(target: ImportTarget, + moduleVarName: LocalName, path: List[String], + used: SimpleState[IsUsed], cancelFun: CancelFun) + private final class LabelInfo( val newName: LabelName, val acceptRecords: Boolean, @@ -4845,27 +4974,43 @@ private[optimizer] object OptimizerCore { val Empty: OptEnv = new OptEnv(None, Map.empty, Map.empty) } - private class Scope(val env: OptEnv, - val implsBeingInlined: Set[Scope.InliningID]) { - def withEnv(env: OptEnv): Scope = - new Scope(env, implsBeingInlined) + private class Scope private ( + val env: OptEnv, + val implsBeingInlined: Set[Scope.InliningID], + val importReplacement: Option[ImportReplacement] + ) { + def withEnv(env: OptEnv): Scope = copy(env = env) def inlining(impl: Scope.InliningID): Scope = { assert(!implsBeingInlined(impl), s"Circular inlining of $impl") - new Scope(env, implsBeingInlined + impl) + copy(implsBeingInlined = implsBeingInlined + impl) } def inlining(impls: Set[Scope.InliningID]): Scope = { val intersection = implsBeingInlined.intersect(impls) assert(intersection.isEmpty, s"Circular inlining of $intersection") - new Scope(env, implsBeingInlined ++ impls) + copy(implsBeingInlined = implsBeingInlined ++ impls) + } + + def withImportReplacement(importReplacement: ImportReplacement): Scope = { + assert(this.importReplacement.isEmpty, "Alreadying replacing " + + s"$this.importReplacement while trying to replace $importReplacement") + copy(importReplacement = Some(importReplacement)) + } + + private def copy( + env: OptEnv = env, + implsBeingInlined: Set[Scope.InliningID] = implsBeingInlined, + importReplacement: Option[ImportReplacement] = importReplacement + ): Scope = { + new Scope(env, implsBeingInlined, importReplacement) } } private object Scope { type InliningID = (List[AllocationSite], AbstractMethodID) - val Empty: Scope = new Scope(OptEnv.Empty, Set.empty) + val Empty: Scope = new Scope(OptEnv.Empty, Set.empty, None) } /** The result of pretransformExpr(). @@ -5182,6 +5327,32 @@ private[optimizer] object OptimizerCore { If(lhs, rhs, BooleanLiteral(false))(BooleanType) } + private object JSImport { + /** Import module and call `callback` with it. */ + def apply(moduleKind: ModuleKind, module: String, callback: Closure)(implicit pos: Position): Tree = { + def genThen(receiver: Tree, callback: Closure): Tree = + JSMethodApply(receiver, StringLiteral("then"), List(callback)) + + val importTree = moduleKind match { + case ModuleKind.NoModule => + throw new AssertionError("Cannot import module in NoModule mode") + + case ModuleKind.ESModule => + JSImportCall(StringLiteral(module)) + + case ModuleKind.CommonJSModule => + val require = + JSFunctionApply(JSGlobalRef("require"), List(StringLiteral(module))) + + val unitPromise = JSMethodApply( + JSGlobalRef("Promise"), StringLiteral("resolve"), List(Undefined())) + genThen(unitPromise, Closure(arrow = true, Nil, Nil, None, require, Nil)) + } + + genThen(importTree, callback) + } + } + /** Creates a new instance of `RuntimeLong` from a record of its `lo` and * `hi` parts. */ @@ -5384,6 +5555,7 @@ private[optimizer] object OptimizerCore { /** Parts of [[GenIncOptimizer#MethodImpl]] with decisions about optimizations. */ abstract class MethodImpl { + def enclosingClassName: ClassName def methodName: MethodName def optimizerHints: OptimizerHints def originalDef: MethodDef @@ -5458,7 +5630,40 @@ private[optimizer] object OptimizerCore { } } - new MethodAttributes(inlineable, shouldInline, isForwarder) + val jsDynImportInlineTarget = body match { + case MaybeUnbox(SelectJSNativeMember(className, MethodIdent(member)), _) => + Some(ImportTarget.Member(className, member)) + + case MaybeUnbox(JSFunctionApply(SelectJSNativeMember(className, + MethodIdent(member)), args), _) if args.forall(isSmallTree(_))=> + Some(ImportTarget.Member(className, member)) + + case MaybeUnbox(LoadJSModule(className), _) => + Some(ImportTarget.Class(className)) + + case MaybeUnbox(JSSelect(LoadJSModule(className), arg), _) if isSmallTree(arg) => + Some(ImportTarget.Class(className)) + + case MaybeUnbox(JSMethodApply(LoadJSModule(className), method, args), _) + if isSmallTree(method) && args.forall(isSmallTree(_)) => + Some(ImportTarget.Class(className)) + + case JSNew(LoadJSConstructor(className), args) if args.forall(isSmallTree(_)) => + Some(ImportTarget.Class(className)) + + case _ => + None + } + + val jsDynImportThunkFor = body match { + case Apply(_, New(clazz, _, _), MethodIdent(target), _) if clazz == enclosingClassName => + Some(target) + + case _ => + None + } + + new MethodAttributes(inlineable, shouldInline, isForwarder, jsDynImportInlineTarget, jsDynImportThunkFor) } } @@ -5472,9 +5677,18 @@ private[optimizer] object OptimizerCore { final case class MethodAttributes private[OptimizerCore] ( private[OptimizerCore] val inlineable: Boolean, private[OptimizerCore] val shouldInline: Boolean, - private[OptimizerCore] val isForwarder: Boolean + private[OptimizerCore] val isForwarder: Boolean, + private[OptimizerCore] val jsDynImportInlineTarget: Option[ImportTarget], + private[OptimizerCore] val jsDynImportThunkFor: Option[MethodName] ) + sealed trait ImportTarget + + object ImportTarget { + case class Member(className: ClassName, member: MethodName) extends ImportTarget + case class Class(className: ClassName) extends ImportTarget + } + private object MaybeUnbox { def unapply(tree: Tree): Some[(Tree, Any)] = tree match { case AsInstanceOf(arg, tpe) => @@ -5497,6 +5711,21 @@ private[optimizer] object OptimizerCore { false } + /** Whether a tree is going to result in a small code size. + * + * This is used to determine whether it is acceptable to move a tree accross + * a dynamic module load boundary. + */ + private def isSmallTree(tree: TreeOrJSSpread): Boolean = tree match { + case _:VarRef | _:Literal => true + case Select(This(), _, _) => true + case UnaryOp(_, lhs) => isSmallTree(lhs) + case BinaryOp(_, lhs, rhs) => isSmallTree(lhs) && isSmallTree(rhs) + case JSUnaryOp(_, lhs) => isSmallTree(lhs) + case JSBinaryOp(_, lhs, rhs) => isSmallTree(lhs) && isSmallTree(rhs) + case _ => false + } + private object SimpleMethodBody { @tailrec final def unapply(body: Tree): Boolean = body match { diff --git a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala index 00137ae1f3..b33238c994 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala @@ -184,7 +184,7 @@ class OptimizerTest { ) for (moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers, TestIRRepo.fulllib)) yield { - assertFalse(moduleSet.modules.flatMap(_.classDefs).exists(_.className == ClassClass)) + assertFalse(findClass(moduleSet, ClassClass).isDefined) } } @@ -216,8 +216,7 @@ class OptimizerTest { ) for (moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers)) yield { - val mainClassDef = moduleSet.modules.flatMap(_.classDefs) - .find(_.className == MainTestClassName).get + val mainClassDef = findClass(moduleSet, MainTestClassName).get assertTrue(mainClassDef.fields.exists { case FieldDef(_, FieldIdent(name), _, _) => name == FieldName("foo") case _ => false @@ -364,12 +363,74 @@ class OptimizerTest { assertEquals("wrong closure params", List(List(x2, x4)), closureParams) } } + + @Test + def testOptimizeDynamicImport(): AsyncResult = await { + val thunkMethodName = m("thunk", Nil, O) + val implMethodName = m("impl", Nil, O) + + val memberMethodName = m("member", Nil, O) + + val SMF = EMF.withNamespace(MemberNamespace.PublicStatic) + + val classDefs = Seq( + mainTestClassDef({ + consoleLog(ApplyDynamicImport(EAF, "Thunk", thunkMethodName, Nil)) + }), + classDef("Thunk", + superClass = Some(ObjectClass), + optimizerHints = EOH.withInline(true), + memberDefs = List( + trivialCtor("Thunk"), + MethodDef(EMF, implMethodName, NON, Nil, AnyType, Some { + SelectJSNativeMember("Holder", memberMethodName) + })(EOH, None), + MethodDef(SMF, thunkMethodName, NON, Nil, AnyType, Some { + val inst = New("Thunk", NoArgConstructorName, Nil) + Apply(EAF, inst, implMethodName, Nil)(AnyType) + })(EOH, None) + ) + ), + classDef("Holder", kind = ClassKind.Interface, + memberDefs = List( + JSNativeMemberDef(SMF, memberMethodName, JSNativeLoadSpec.Import("foo", List("bar"))) + ) + ) + ) + + val linkerConfig = StandardConfig() + .withModuleKind(ModuleKind.ESModule) + + for { + moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers, + linkerConfig = linkerConfig) + } yield { + assertFalse(findClass(moduleSet, "Thunk").isDefined) + + var foundJSImport = false + val main = findClass(moduleSet, MainTestClassName).get + val traverser = new Traverser { + override def traverse(tree: Tree): Unit = tree match { + case tree: ApplyDynamicImport => fail(s"found ApplyDynamicImport " + tree) + case tree: JSImportCall => foundJSImport = true + case tree => super.traverse(tree) + } + } + + main.methods.foreach(v => traverser.traverseMemberDef(v.value)) + + assertTrue(foundJSImport) + } + } } object OptimizerTest { private val cloneMethodName = m("clone", Nil, O) private val witnessMethodName = m("witness", Nil, O) + private def findClass(moduleSet: ModuleSet, name: ClassName): Option[LinkedClass] = + moduleSet.modules.flatMap(_.classDefs).find(_.className == name) + private final class StoreModuleSetLinkerBackend( originalBackend: LinkerBackend) extends LinkerBackend { @@ -397,17 +458,12 @@ object OptimizerTest { } def linkToModuleSet(classDefs: Seq[ClassDef], - moduleInitializers: List[ModuleInitializer])( - implicit ec: ExecutionContext): Future[ModuleSet] = { - - linkToModuleSet(classDefs, moduleInitializers, TestIRRepo.minilib) - } - - def linkToModuleSet(classDefs: Seq[ClassDef], - moduleInitializers: List[ModuleInitializer], stdlib: Future[Seq[IRFile]])( + moduleInitializers: List[ModuleInitializer], + stdlib: Future[Seq[IRFile]] = TestIRRepo.minilib, + linkerConfig: StandardConfig = StandardConfig())( implicit ec: ExecutionContext): Future[ModuleSet] = { - val config = StandardConfig().withCheckIR(true) + val config = linkerConfig.withCheckIR(true) val frontend = StandardLinkerFrontend(config) val backend = new StoreModuleSetLinkerBackend(StandardLinkerBackend(config)) val linker = StandardLinkerImpl(frontend, backend) diff --git a/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala b/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala index 9f30a6ab26..dd55c0eae1 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/testutils/TestIRBuilder.scala @@ -44,19 +44,21 @@ object TestIRBuilder { MethodName(name, paramTypeRefs, resultTypeRef) def classDef( - className: ClassName, - kind: ClassKind = ClassKind.Class, - jsClassCaptures: Option[List[ParamDef]] = None, - superClass: Option[ClassName] = None, - interfaces: List[ClassName] = Nil, - jsSuperClass: Option[Tree] = None, - jsNativeLoadSpec: Option[JSNativeLoadSpec] = None, - memberDefs: List[MemberDef] = Nil, - topLevelExportDefs: List[TopLevelExportDef] = Nil): ClassDef = { + className: ClassName, + kind: ClassKind = ClassKind.Class, + jsClassCaptures: Option[List[ParamDef]] = None, + superClass: Option[ClassName] = None, + interfaces: List[ClassName] = Nil, + jsSuperClass: Option[Tree] = None, + jsNativeLoadSpec: Option[JSNativeLoadSpec] = None, + memberDefs: List[MemberDef] = Nil, + topLevelExportDefs: List[TopLevelExportDef] = Nil, + optimizerHints: OptimizerHints = EOH + ): ClassDef = { val notHashed = ClassDef(ClassIdent(className), NON, kind, jsClassCaptures, superClass.map(ClassIdent(_)), interfaces.map(ClassIdent(_)), jsSuperClass, jsNativeLoadSpec, memberDefs, topLevelExportDefs)( - EOH) + optimizerHints) Hashers.hashClassDef(notHashed) } diff --git a/test-suite/js/src/test/require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTest.scala b/test-suite/js/src/test/require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTest.scala index f1122cf470..3166d334bc 100644 --- a/test-suite/js/src/test/require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTest.scala +++ b/test-suite/js/src/test/require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTest.scala @@ -132,6 +132,71 @@ class SJSDynamicImportTest { } } + @Test + def optimizedNativeMethod(): AsyncResult = await { + val promise = js.dynamicImport { + ModulesTest.NativeMembers.ssum(2, 3) + } + + promise.toFuture.map { x => + assertEquals(13, x) + } + } + + @Test + def optimizedNativeProperty(): AsyncResult = await { + val promise = js.dynamicImport { + ModulesTest.NativeMembers.strConstant + } + + promise.toFuture.map { x => + assertEquals("value", x) + } + } + + @Test + def optimizedNativeModule(): AsyncResult = await { + val promise = js.dynamicImport { ModulesTest.MyBox } + + promise.toFuture.map { x => + assertSame(ModulesTest.MyBox, x) + } + } + + @Test + def optimizedNativeModuleMethod(): AsyncResult = await { + val promise = js.dynamicImport { + ModulesTest.NamespaceImport.ssum(1, 2) + } + + promise.toFuture.map { x => + assertEquals(5, x) + } + } + + @Test + def optimizedNativeModuleProperty(): AsyncResult = await { + val promise = js.dynamicImport { + ModulesTest.NamespaceImport.strConstantAsDef + } + + promise.toFuture.map { x => + assertEquals("value", x) + } + } + + @Test + def optimizedNativeNew(): AsyncResult = await { + val promise = js.dynamicImport { + new ModulesTest.MyBox(2L) + } + + promise.toFuture.map { b => + assertEquals(2L, b.get()) + assertTrue(b.isInstanceOf[ModulesTest.MyBox[_]]) + } + } + @Test def nested(): AsyncResult = await { // Ludicrously complicated nested dynamic imports. From 9def628dbb6662b507fd31505b6d238485abff13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 29 Mar 2022 15:29:33 +0200 Subject: [PATCH 17/20] Fix #4657: Implement UUID.randomUUID() using java.security.SecureRandom. Since Scala.js core does not implement `SecureRandom`, this means that `randomUUID()` will fail to link unless `SecureRandom` is otherwise provided. We move the tests for `randomUUID()` in the test-suite-ex, and we implement a fake `SecureRandom` in the javalib-ext-dummies for testing purposes. --- .../scala/java/security/SecureRandom.scala | 19 ++++++++++ javalib/src/main/scala/java/util/UUID.scala | 23 +++++++++--- .../testsuite/javalib/util/UUIDTestEx.scala | 37 +++++++++++++++++++ .../testsuite/javalib/util/UUIDTest.scala | 6 --- 4 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 javalib-ext-dummies/src/main/scala/java/security/SecureRandom.scala create mode 100644 test-suite-ex/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTestEx.scala diff --git a/javalib-ext-dummies/src/main/scala/java/security/SecureRandom.scala b/javalib-ext-dummies/src/main/scala/java/security/SecureRandom.scala new file mode 100644 index 0000000000..47f9555fbe --- /dev/null +++ b/javalib-ext-dummies/src/main/scala/java/security/SecureRandom.scala @@ -0,0 +1,19 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package java.security + +/** Fake implementation of `SecureRandom` that is not actually secure at all. + * + * It directly delegates to `java.util.Random`. + */ +class SecureRandom extends java.util.Random diff --git a/javalib/src/main/scala/java/util/UUID.scala b/javalib/src/main/scala/java/util/UUID.scala index 9daedcd2b9..ab08fa3fea 100644 --- a/javalib/src/main/scala/java/util/UUID.scala +++ b/javalib/src/main/scala/java/util/UUID.scala @@ -136,13 +136,26 @@ object UUID { private final val NameBased = 3 private final val Random = 4 - private lazy val rng = new Random() // TODO Use java.security.SecureRandom + // Typed as `Random` so that the IR typechecks when SecureRandom is not available + private lazy val csprng: Random = new java.security.SecureRandom() + private lazy val randomUUIDBuffer: Array[Byte] = new Array[Byte](16) def randomUUID(): UUID = { - val i1 = rng.nextInt() - val i2 = (rng.nextInt() & ~0x0000f000) | 0x00004000 - val i3 = (rng.nextInt() & ~0xc0000000) | 0x80000000 - val i4 = rng.nextInt() + val buffer = randomUUIDBuffer // local copy + + /* We use nextBytes() because that is the primitive of most secure RNGs, + * and therefore it allows to perform a unique call to the underlying + * secure RNG. + */ + csprng.nextBytes(randomUUIDBuffer) + + @inline def intFromBuffer(i: Int): Int = + (buffer(i) << 24) | ((buffer(i + 1) & 0xff) << 16) | ((buffer(i + 2) & 0xff) << 8) | (buffer(i + 3) & 0xff) + + val i1 = intFromBuffer(0) + val i2 = (intFromBuffer(4) & ~0x0000f000) | 0x00004000 + val i3 = (intFromBuffer(8) & ~0xc0000000) | 0x80000000 + val i4 = intFromBuffer(12) new UUID(i1, i2, i3, i4, null, null) } diff --git a/test-suite-ex/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTestEx.scala b/test-suite-ex/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTestEx.scala new file mode 100644 index 0000000000..44a9cde78a --- /dev/null +++ b/test-suite-ex/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTestEx.scala @@ -0,0 +1,37 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.testsuite.javalib.util + +import java.util.UUID + +import org.junit.Test +import org.junit.Assert._ + +/** Additional tests for `java.util.UUID` that require + * `java.security.SecureRandom`. + */ +class UUIDTestEx { + + @Test def randomUUID(): Unit = { + val uuid1 = UUID.randomUUID() + assertEquals(2, uuid1.variant()) + assertEquals(4, uuid1.version()) + + val uuid2 = UUID.randomUUID() + assertEquals(2, uuid2.variant()) + assertEquals(4, uuid2.version()) + + assertNotEquals(uuid1, uuid2) + } + +} diff --git a/test-suite/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTest.scala b/test-suite/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTest.scala index eec4705b6d..dbc078b682 100644 --- a/test-suite/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTest.scala +++ b/test-suite/shared/src/test/scala/org/scalajs/testsuite/javalib/util/UUIDTest.scala @@ -142,12 +142,6 @@ class UUIDTest { new UUID(0x0000000000001000L, 0x8000000000000000L).toString) } - @Test def randomUUID(): Unit = { - val uuid = UUID.randomUUID() - assertEquals(2, uuid.variant()) - assertEquals(4, uuid.version()) - } - @Test def fromString(): Unit = { val uuid1 = UUID.fromString("f81d4fae-7dec-11d0-a765-00a0c91e6bf6") assertTrue(uuid1.equals(new UUID(0xf81d4fae7dec11d0L, 0xa76500a0c91e6bf6L))) From c362d8b578de55a61cba931219b0769e5ca2e110 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 27 Mar 2022 09:33:38 +0200 Subject: [PATCH 18/20] Rename {Min,Max}ModuleAnalyzer to {Smallest,Fewest}ModulesAnalyzer The min/max naming is an artifact of the initial creation. There is no point in having different names in the implementation than in the interface. --- .../org/scalajs/linker/frontend/LinkerFrontendImpl.scala | 4 ++-- ...xModuleAnalyzer.scala => FewestModulesAnalyzer.scala} | 6 +++--- .../linker/frontend/modulesplitter/ModuleSplitter.scala | 9 +++++---- ...oduleAnalyzer.scala => SmallestModulesAnalyzer.scala} | 6 +++--- ...ittingTest.scala => FewestModulesSplittingTest.scala} | 2 +- ...tingTest.scala => SmallestModulesSplittingTest.scala} | 2 +- 6 files changed, 15 insertions(+), 14 deletions(-) rename linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/{MaxModuleAnalyzer.scala => FewestModulesAnalyzer.scala} (98%) rename linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/{MinModuleAnalyzer.scala => SmallestModulesAnalyzer.scala} (96%) rename linker/shared/src/test/scala/org/scalajs/linker/{MaxModuleSplittingTest.scala => FewestModulesSplittingTest.scala} (98%) rename linker/shared/src/test/scala/org/scalajs/linker/{MinModuleSplittingTest.scala => SmallestModulesSplittingTest.scala} (98%) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala index c094f360f7..9ffa84e9c0 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala @@ -46,8 +46,8 @@ final class LinkerFrontendImpl private (config: LinkerFrontendImpl.Config) private[this] val refiner: Refiner = new Refiner(config.commonConfig) private[this] val splitter: ModuleSplitter = config.moduleSplitStyle match { - case ModuleSplitStyle.FewestModules => ModuleSplitter.maxSplitter() - case ModuleSplitStyle.SmallestModules => ModuleSplitter.minSplitter() + case ModuleSplitStyle.FewestModules => ModuleSplitter.fewestModules() + case ModuleSplitStyle.SmallestModules => ModuleSplitter.smallestModules() } /** Link and optionally optimize the given IR to a diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/MaxModuleAnalyzer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/FewestModulesAnalyzer.scala similarity index 98% rename from linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/MaxModuleAnalyzer.scala rename to linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/FewestModulesAnalyzer.scala index d51416843b..6151012c87 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/MaxModuleAnalyzer.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/FewestModulesAnalyzer.scala @@ -30,8 +30,8 @@ import org.scalajs.linker.standard.ModuleSet.ModuleID * each public module it can be reached by. We then create a module for each * distinct set of tags. */ -private[modulesplitter] final class MaxModuleAnalyzer extends ModuleAnalyzer { - import MaxModuleAnalyzer._ +private[modulesplitter] final class FewestModulesAnalyzer extends ModuleAnalyzer { + import FewestModulesAnalyzer._ def analyze(info: ModuleAnalyzer.DependencyInfo): ModuleAnalyzer.Analysis = { val hasDynDeps = info.classDependencies.exists(_._2.dynamicDependencies.nonEmpty) @@ -61,7 +61,7 @@ private[modulesplitter] final class MaxModuleAnalyzer extends ModuleAnalyzer { } } -private object MaxModuleAnalyzer { +private object FewestModulesAnalyzer { private final class SingleModuleAnalysis(moduleID: ModuleID) extends ModuleAnalyzer.Analysis { diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala index 190227ae32..ac609fe020 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala @@ -177,11 +177,12 @@ final class ModuleSplitter private (analyzer: ModuleAnalyzer) { } object ModuleSplitter { - def minSplitter(): ModuleSplitter = - new ModuleSplitter(new MinModuleAnalyzer()) - def maxSplitter(): ModuleSplitter = - new ModuleSplitter(new MaxModuleAnalyzer()) + def smallestModules(): ModuleSplitter = + new ModuleSplitter(new SmallestModulesAnalyzer()) + + def fewestModules(): ModuleSplitter = + new ModuleSplitter(new FewestModulesAnalyzer()) private class ModuleBuilder(id: ModuleID) { val internalDependencies: Builder[ModuleID, Set[ModuleID]] = Set.newBuilder diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/MinModuleAnalyzer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallestModulesAnalyzer.scala similarity index 96% rename from linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/MinModuleAnalyzer.scala rename to linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallestModulesAnalyzer.scala index 893f00a03a..c99177e69c 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/MinModuleAnalyzer.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallestModulesAnalyzer.scala @@ -25,15 +25,15 @@ import org.scalajs.linker.standard.ModuleSet.ModuleID * In practice, this means it generates a module per strongly connected * component of the (static) dependency graph. */ -private[modulesplitter] final class MinModuleAnalyzer extends ModuleAnalyzer { +private[modulesplitter] final class SmallestModulesAnalyzer extends ModuleAnalyzer { def analyze(info: ModuleAnalyzer.DependencyInfo): ModuleAnalyzer.Analysis = { - val run = new MinModuleAnalyzer.Run(info) + val run = new SmallestModulesAnalyzer.Run(info) run.analyze() run } } -private object MinModuleAnalyzer { +private object SmallestModulesAnalyzer { private final class Node(val className: ClassName, val index: Int) { var lowlink: Int = index var moduleIndex: Int = -1 diff --git a/linker/shared/src/test/scala/org/scalajs/linker/MaxModuleSplittingTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/FewestModulesSplittingTest.scala similarity index 98% rename from linker/shared/src/test/scala/org/scalajs/linker/MaxModuleSplittingTest.scala rename to linker/shared/src/test/scala/org/scalajs/linker/FewestModulesSplittingTest.scala index c9c5017786..b2b9693abe 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/MaxModuleSplittingTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/FewestModulesSplittingTest.scala @@ -28,7 +28,7 @@ import org.scalajs.linker.interface._ import org.scalajs.linker.testutils.LinkingUtils._ import org.scalajs.linker.testutils.TestIRBuilder._ -class MaxModuleSplittingTest { +class FewestModulesSplittingTest { import scala.concurrent.ExecutionContext.Implicits.global @Test diff --git a/linker/shared/src/test/scala/org/scalajs/linker/MinModuleSplittingTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/SmallestModulesSplittingTest.scala similarity index 98% rename from linker/shared/src/test/scala/org/scalajs/linker/MinModuleSplittingTest.scala rename to linker/shared/src/test/scala/org/scalajs/linker/SmallestModulesSplittingTest.scala index fdcd9d546a..3ff90b20b9 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/MinModuleSplittingTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/SmallestModulesSplittingTest.scala @@ -27,7 +27,7 @@ import org.scalajs.linker.interface._ import org.scalajs.linker.testutils.LinkingUtils._ import org.scalajs.linker.testutils.TestIRBuilder._ -class MinModuleSplittingTest { +class SmallestModulesSplittingTest { import scala.concurrent.ExecutionContext.Implicits.global /** Smoke test to ensure modules do not get merged too much. */ From af9df79143303af270bfabdc742f79d03019e360 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 27 Mar 2022 10:24:34 +0200 Subject: [PATCH 19/20] Fix #4327: SmallModulesFor module split style --- Jenkinsfile | 14 ++ .../linker/interface/ModuleSplitStyle.scala | 38 ++- .../interface/ModuleSplitStyleTest.scala | 60 +++++ .../linker/frontend/LinkerFrontendImpl.scala | 5 +- .../FewestModulesAnalyzer.scala | 203 +-------------- .../frontend/modulesplitter/ModuleIDs.scala | 71 ++++++ .../modulesplitter/ModuleSplitter.scala | 3 + .../SmallModulesForAnalyzer.scala | 108 ++++++++ .../SmallestModulesAnalyzer.scala | 145 +---------- .../modulesplitter/StrongConnect.scala | 139 +++++++++++ .../frontend/modulesplitter/Tagger.scala | 235 ++++++++++++++++++ .../org/scalajs/linker/OptimizerTest.scala | 55 +--- .../linker/SmallModulesForSplittingTest.scala | 95 +++++++ .../linker/testutils/LinkingUtils.scala | 48 ++++ 14 files changed, 829 insertions(+), 390 deletions(-) create mode 100644 linker-interface/shared/src/test/scala/org/scalajs/linker/interface/ModuleSplitStyleTest.scala create mode 100644 linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleIDs.scala create mode 100644 linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallModulesForAnalyzer.scala create mode 100644 linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/StrongConnect.scala create mode 100644 linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/Tagger.scala create mode 100644 linker/shared/src/test/scala/org/scalajs/linker/SmallModulesForSplittingTest.scala diff --git a/Jenkinsfile b/Jenkinsfile index 4d1ff74dd5..3bdcde1623 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -132,6 +132,10 @@ def Tasks = [ 'set scalaJSLinkerConfig in helloworld.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallestModules))' \ 'set scalaJSLinkerConfig in helloworld.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \ helloworld$v/run && + sbtretry ++$scala \ + 'set scalaJSLinkerConfig in helloworld.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallModulesFor(List("helloworld"))))' \ + 'set scalaJSLinkerConfig in helloworld.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \ + helloworld$v/run && sbtretry ++$scala \ 'set scalaJSLinkerConfig in helloworld.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \ 'set scalaJSStage in Global := FullOptStage' \ @@ -162,6 +166,12 @@ def Tasks = [ reversi$v/fastLinkJS \ reversi$v/fullLinkJS \ reversi$v/clean && + sbtretry ++$scala \ + 'set scalaJSLinkerConfig in reversi.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallModulesFor(List("reversi"))))' \ + 'set scalaJSLinkerConfig in reversi.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \ + reversi$v/fastLinkJS \ + reversi$v/fullLinkJS \ + reversi$v/clean && sbtretry ++$scala \ reversi$v/fastLinkJS \ reversi$v/fullLinkJS \ @@ -248,6 +258,10 @@ def Tasks = [ 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallestModules))' \ 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \ ++$scala $testSuite$v/test && + sbtretry \ + 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallModulesFor(List("org.scalajs.testsuite"))))' \ + 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \ + ++$scala $testSuite$v/test && sbtretry \ 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \ 'set scalaJSStage in Global := FullOptStage' \ diff --git a/linker-interface/shared/src/main/scala/org/scalajs/linker/interface/ModuleSplitStyle.scala b/linker-interface/shared/src/main/scala/org/scalajs/linker/interface/ModuleSplitStyle.scala index 031ead0fbc..06e1c685b8 100644 --- a/linker-interface/shared/src/main/scala/org/scalajs/linker/interface/ModuleSplitStyle.scala +++ b/linker-interface/shared/src/main/scala/org/scalajs/linker/interface/ModuleSplitStyle.scala @@ -12,6 +12,9 @@ package org.scalajs.linker.interface +import java.nio.CharBuffer +import java.nio.charset.{StandardCharsets, CharacterCodingException} + /** How to split the output into modules. */ abstract class ModuleSplitStyle private () @@ -27,14 +30,45 @@ object ModuleSplitStyle { /** Make modules as small as possible. */ case object SmallestModules extends ModuleSplitStyle + /** Mix between [[FewestModules]] / [[SmallestModules]] + * + * * Make modules as small as possible for all classes in any of [[packages]] + * (or subpackages thereof). + * * Make as few modules as possible for everything else (while not including + * unnecessary code). + * + * @note In order to avoid ambiguity between packages and classes (and keep a + * simple interface), selecting individual classes is not supported. + */ + final case class SmallModulesFor(packages: List[String]) extends ModuleSplitStyle { + require(packages.nonEmpty, "must have at least one package") + packages.foreach(p => require(isValidPackage(p), s"invalid package name $p")) + } + private[interface] implicit object ModuleSplitStyleFingerprint extends Fingerprint[ModuleSplitStyle] { override def fingerprint(moduleSplitStyle: ModuleSplitStyle): String = { moduleSplitStyle match { - case FewestModules => "FewestModules" - case SmallestModules => "SmallestModules" + case FewestModules => "FewestModules" + case SmallestModules => "SmallestModules" + case SmallModulesFor(packages) => s"SmallModulesFor($packages)" } } } + + private def isValidPackage(pkg: String): Boolean = { + pkg.nonEmpty && isValidUTF16(pkg) && + List(':', '[', '/').forall(!pkg.contains(_)) && + pkg.split("\\.", -1).forall(_.nonEmpty) + } + + private def isValidUTF16(str: String): Boolean = { + try { + StandardCharsets.UTF_16.newEncoder().encode(CharBuffer.wrap(str)) + true + } catch { + case _: CharacterCodingException => false + } + } } diff --git a/linker-interface/shared/src/test/scala/org/scalajs/linker/interface/ModuleSplitStyleTest.scala b/linker-interface/shared/src/test/scala/org/scalajs/linker/interface/ModuleSplitStyleTest.scala new file mode 100644 index 0000000000..2fe545c164 --- /dev/null +++ b/linker-interface/shared/src/test/scala/org/scalajs/linker/interface/ModuleSplitStyleTest.scala @@ -0,0 +1,60 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.linker.interface + +import org.junit.Test +import org.junit.Assert._ + +class ModuleSplitStyleSmallModulesForTest { + import ModuleSplitStyle.SmallModulesFor + + @Test + def acceptPackage(): Unit = + SmallModulesFor(List("a.b.c")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectNoPackage(): Unit = + SmallModulesFor(List()) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectEmptyPackage(): Unit = + SmallModulesFor(List("")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectTrailingDot(): Unit = + SmallModulesFor(List("a.b.c.")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectLeadingDot(): Unit = + SmallModulesFor(List(".a.b.c")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectDoubleDot(): Unit = + SmallModulesFor(List("a.b..c")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectSlash(): Unit = + SmallModulesFor(List("a.b/.c")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectOpenBracket(): Unit = + SmallModulesFor(List("a.b[.c")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectColon(): Unit = + SmallModulesFor(List("a.b:.c")) + + @Test(expected = classOf[IllegalArgumentException]) + def rejectUnpairedSurrogate(): Unit = + SmallModulesFor(List("a.\ud800.c")) +} diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala index 9ffa84e9c0..58d0f258aa 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/LinkerFrontendImpl.scala @@ -46,8 +46,9 @@ final class LinkerFrontendImpl private (config: LinkerFrontendImpl.Config) private[this] val refiner: Refiner = new Refiner(config.commonConfig) private[this] val splitter: ModuleSplitter = config.moduleSplitStyle match { - case ModuleSplitStyle.FewestModules => ModuleSplitter.fewestModules() - case ModuleSplitStyle.SmallestModules => ModuleSplitter.smallestModules() + case ModuleSplitStyle.FewestModules => ModuleSplitter.fewestModules() + case ModuleSplitStyle.SmallestModules => ModuleSplitter.smallestModules() + case ModuleSplitStyle.SmallModulesFor(packages) => ModuleSplitter.smallModulesFor(packages) } /** Link and optionally optimize the given IR to a diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/FewestModulesAnalyzer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/FewestModulesAnalyzer.scala index 6151012c87..f5b2522705 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/FewestModulesAnalyzer.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/FewestModulesAnalyzer.scala @@ -12,15 +12,7 @@ package org.scalajs.linker.frontend.modulesplitter -import scala.annotation.tailrec - -import scala.collection.immutable -import scala.collection.mutable - -import java.nio.charset.StandardCharsets - import org.scalajs.ir.Names.ClassName -import org.scalajs.ir.SHA1 import org.scalajs.linker.standard.ModuleSet.ModuleID /** Build fewest (largest) possible modules (without reaching unnecessary code). @@ -40,25 +32,14 @@ private[modulesplitter] final class FewestModulesAnalyzer extends ModuleAnalyzer // Fast path. new SingleModuleAnalysis(info.publicModuleDependencies.head._1) } else { - val prefix = internalModuleIDPrefix(info.publicModuleDependencies.keys) + val prefix = ModuleIDs.freeInternalPrefix( + avoid = info.publicModuleDependencies.keys) + val moduleMap = new Tagger(info).tagAll(prefix) new FullAnalysis(moduleMap) } } - - /** Create a prefix for internal modules. - * - * Chosen such that it is not a prefix of any public module ID. - * This ensures that a generated internal module ID never collides with a - * public module ID. - */ - private def internalModuleIDPrefix(publicIDs: Iterable[ModuleID]): String = { - Iterator - .iterate("internal-")(_ + "-") - .find(p => !publicIDs.exists(_.id.startsWith(p))) - .get - } } private object FewestModulesAnalyzer { @@ -74,182 +55,4 @@ private object FewestModulesAnalyzer { def moduleForClass(className: ClassName): Option[ModuleID] = map.get(className) } - - /** Tagger performs the actual grouping of classes into modules. - * - * To group classes into modules appropriately, we want to know for - * each class, "how" it can be reached. In practice, this means we - * record the path from the original public module and every - * dynamic import hop we made. - * - * Of all these paths, we only care about the "simplest" ones. Or - * more formally, the minimum prefixes of all paths. For example, - * if a class is reachable by the following paths: - * - * - a -> b - * - a -> b -> c - * - d -> c - * - d - * - * We really only care about: - * - * - a -> b - * - d - * - * Because if we reach the class through a path that goes through - * `c`, it is necessarily already loaded. - * - * Once we have obtained this minimal set of paths, we use the last - * element of each path to determine the final module - * grouping. This is because these have an actual static dependency - * on the node in question. - * - * Merging these tags into a single `ModuleID` is delegated to the - * caller. - */ - private final class Tagger(infos: ModuleAnalyzer.DependencyInfo) { - private[this] val allPaths = mutable.Map.empty[ClassName, Paths] - - def tagAll(internalModuleIDPrefix: String): scala.collection.Map[ClassName, ModuleID] = { - tagEntryPoints() - allPaths.map { case (className, paths) => - className -> paths.moduleID(internalModuleIDPrefix) - } - } - - private def tag(className: ClassName, pathRoot: ModuleID, pathSteps: List[ClassName]): Unit = { - val updated = allPaths - .getOrElseUpdate(className, new Paths) - .put(pathRoot, pathSteps) - - if (updated) { - val classInfo = infos.classDependencies(className) - classInfo - .staticDependencies - .foreach(tag(_, pathRoot, pathSteps)) - - classInfo - .dynamicDependencies - .foreach(c => tag(c, pathRoot, pathSteps :+ c)) - } - } - - private def tagEntryPoints(): Unit = { - for { - (moduleID, deps) <- infos.publicModuleDependencies - className <- deps - } { - tag(className, moduleID, Nil) - } - } - } - - /** "Interesting" paths that can lead to a given class. - * - * "Interesting" in this context means: - * - All direct paths from a public dependency. - * - All non-empty, mutually prefix-free paths of dynamic import hops. - */ - private final class Paths { - private val direct = mutable.Set.empty[ModuleID] - private val dynamic = mutable.Map.empty[ModuleID, DynamicPaths] - - def put(pathRoot: ModuleID, pathSteps: List[ClassName]): Boolean = { - if (pathSteps.isEmpty) { - direct.add(pathRoot) - } else { - dynamic - .getOrElseUpdate(pathRoot, new DynamicPaths) - .put(pathSteps) - } - } - - def moduleID(internalModuleIDPrefix: String): ModuleID = { - if (direct.size == 1 && dynamic.isEmpty) { - /* Class is only used by a single public module. Put it there. - * - * Note that we must not do this if there are any dynamic modules - * requiring this class. Otherwise, the dynamically loaded module - * will try to import the public module (but importing public modules is - * forbidden). - */ - direct.head - } else { - /* Class is used by multiple public modules and/or dynamic edges. - * Create a module ID grouping it with other classes that have the same - * dependees. - */ - val digestBuilder = new SHA1.DigestBuilder - - // Public modules using this. - for (id <- direct.toList.sortBy(_.id)) - digestBuilder.update(id.id.getBytes(StandardCharsets.UTF_8)) - - // Dynamic modules using this. - for (className <- dynamicEnds) - digestBuilder.updateUTF8String(className.encoded) - - // Build a hex string of the hash with the right prefix. - @inline def hexDigit(digit: Int): Char = - Character.forDigit(digit & 0x0f, 16) - - val id = new java.lang.StringBuilder(internalModuleIDPrefix) - - for (b <- digestBuilder.finalizeDigest()) { - id.append(hexDigit(b >> 4)) - id.append(hexDigit(b)) - } - - ModuleID(id.toString()) - } - } - - private def dynamicEnds: immutable.SortedSet[ClassName] = { - val builder = immutable.SortedSet.newBuilder[ClassName] - /* We ignore paths that originate in a module that imports this class - * directly: They are irrelevant for the final ID. - * - * However, they are important to ensure we do not attempt to import a - * public module (see the comment in moduleID); therefore, we only filter - * them here. - */ - for ((h, t) <- dynamic if !direct.contains(h)) - t.ends(builder) - builder.result() - } - } - - /** Set of shortest, mutually prefix-free paths of dynamic import hops */ - private final class DynamicPaths { - private val content = mutable.Map.empty[ClassName, DynamicPaths] - - @tailrec - def put(path: List[ClassName]): Boolean = { - val h :: t = path - - if (content.get(h).exists(_.content.isEmpty)) { - // shorter or equal path already exists. - false - } else if (t.isEmpty) { - // the path we put stops here, prune longer paths (if any). - content.put(h, new DynamicPaths) - true - } else { - // there are other paths, recurse. - content - .getOrElseUpdate(h, new DynamicPaths) - .put(t) - } - } - - /** Populates `builder` with the ends of all paths. */ - def ends(builder: mutable.Builder[ClassName, Set[ClassName]]): Unit = { - for ((h, t) <- content) { - if (t.content.isEmpty) - builder += h - else - t.ends(builder) - } - } - } } diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleIDs.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleIDs.scala new file mode 100644 index 0000000000..888b2b8d6e --- /dev/null +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleIDs.scala @@ -0,0 +1,71 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.linker.frontend.modulesplitter + +import org.scalajs.ir.Names.{ClassName, ObjectClass} +import org.scalajs.linker.standard.ModuleSet.ModuleID + +/** Helpers to create internal ModulesIDs */ +private object ModuleIDs { + + /** Picks a representative from a list of classes. + * + * Guarantees to return the same value independent of the order of [[names]]. + */ + def representativeClass(names: List[ClassName]): ClassName = { + require(names.nonEmpty) + + /* Take the lexicographically smallest name as a stable name of the + * module, with the exception of j.l.Object which identifies the root + * module. + * + * We do this, because it is simple and stable (i.e. does not depend + * on traversal order). + */ + if (names.contains(ObjectClass)) ObjectClass + else names.min + } + + /** Builds an ID for the class with name [[name]]. + * + * The result is guaranteed to be: + * - Different from any ModuleID in [[avoid]]. + * - Different for each ClassName. + * - Deterministic. + */ + def forClassName(avoid: Set[ModuleID], name: ClassName): ModuleID = { + /* Build a module ID that doesn't collide with others. + * + * We observe: + * - Class names are unique, so they never collide with each other. + * - Appending a dot ('.') to a class name results in an illegal class name. + * + * So we append dots until we hit a ModuleID not used by a public module. + * + * Note that this is stable, because it does not depend on the order we + * iterate over nodes. + */ + var moduleID = ModuleID(name.nameString) + while (avoid.contains(moduleID)) + moduleID = ModuleID(moduleID.id + ".") + moduleID + } + + /** Creates a prefix that is not a prefix of any of the IDs in [[avoid]] */ + def freeInternalPrefix(avoid: Iterable[ModuleID]): String = { + Iterator + .iterate("internal-")(_ + "-") + .find(p => !avoid.exists(_.id.startsWith(p))) + .get + } +} diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala index ac609fe020..579d440265 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/ModuleSplitter.scala @@ -184,6 +184,9 @@ object ModuleSplitter { def fewestModules(): ModuleSplitter = new ModuleSplitter(new FewestModulesAnalyzer()) + def smallModulesFor(packages: List[String]): ModuleSplitter = + new ModuleSplitter(new SmallModulesForAnalyzer(packages.map(ClassName(_)))) + private class ModuleBuilder(id: ModuleID) { val internalDependencies: Builder[ModuleID, Set[ModuleID]] = Set.newBuilder val externalDependencies: Builder[String, Set[String]] = Set.newBuilder diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallModulesForAnalyzer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallModulesForAnalyzer.scala new file mode 100644 index 0000000000..dbf5fcdff9 --- /dev/null +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallModulesForAnalyzer.scala @@ -0,0 +1,108 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.linker.frontend.modulesplitter + +import scala.collection +import scala.collection.mutable + +import org.scalajs.ir.Names.ClassName +import org.scalajs.linker.standard.ModuleSet.ModuleID + +/** Build smallest modules for packages, largest modules for the rest. + * + * For all classes matching any of packages (prefix match), this generates one + * module per class, except when there are circular dependencies. + * + * For all other classes, it creates large modules (while avoiding dependency + * cycles due to the grouping of the small modules). + * + * All in all, this is essentially the [[SmallModulesAnalyzer]] followed by the + * [[FewestModulesAnalyzer]]. + */ +private final class SmallModulesForAnalyzer( + packages: List[ClassName]) extends ModuleAnalyzer { + def analyze(info: ModuleAnalyzer.DependencyInfo): ModuleAnalyzer.Analysis = { + val (targetClassToRepr, reprToModuleID) = smallRun(info, packages) + + val prefix = ModuleIDs.freeInternalPrefix( + info.publicModuleDependencies.keys ++ reprToModuleID.values) + + val largeModuleMap = + new Tagger(info, excludedClasses = targetClassToRepr.keySet).tagAll(prefix) + + new SmallModulesForAnalyzer.Analysis(targetClassToRepr, reprToModuleID, largeModuleMap) + } + + private def smallRun(info: ModuleAnalyzer.DependencyInfo, packages: List[ClassName]) = { + val run = new SmallModulesForAnalyzer.SmallRun(info, packages) + run.analyze() + + // Only return relevant fields for better GC. + (run.targetClassToRepr, run.reprToModuleID) + } +} + +private object SmallModulesForAnalyzer { + + private final class Analysis(targetClassToRepr: collection.Map[ClassName, ClassName], + reprToModuleID: collection.Map[ClassName, ModuleID], + largeModuleMap: collection.Map[ClassName, ModuleID]) extends ModuleAnalyzer.Analysis { + def moduleForClass(className: ClassName): Option[ModuleID] = { + largeModuleMap.get(className).orElse { + targetClassToRepr.get(className).map(reprToModuleID(_)) + } + } + } + + private final class SmallRun(info: ModuleAnalyzer.DependencyInfo, + packages: List[ClassName]) extends StrongConnect(info) { + + /* We expect this to contain relatively few classes. + * + * So instead of keeping the underlying graph and relying on [[moduleIndex]], + * we create this ad-hoc map. + */ + val targetClassToRepr = mutable.Map.empty[ClassName, ClassName] + + val reprToModuleID = mutable.Map.empty[ClassName, ModuleID] + + protected def emitModule(moduleIndex: Int, classNames: List[ClassName]): Unit = { + // Target classes contained in this strongly connected component. + val targetNames = classNames.filter(clazz => packages.exists(inPackage(clazz, _))) + + if (targetNames.nonEmpty) { + val repr = ModuleIDs.representativeClass(targetNames) + val id = ModuleIDs.forClassName(info.publicModuleDependencies.keySet, repr) + reprToModuleID(repr) = id + for (className <- classNames) + targetClassToRepr(className) = repr + } + } + + private def inPackage(clazz: ClassName, pkg: ClassName): Boolean = { + val clazzEnc = clazz.encoded + val clazzLen = clazzEnc.length + val pkgEnc = pkg.encoded + val pkgLen = pkgEnc.length + + if (clazzLen > pkgLen && clazzEnc(pkgLen) == '.') { + var i = 0 + while (i != pkgLen && clazzEnc(i) == pkgEnc(i)) + i += 1 + i == pkgLen + } else { + false + } + } + } +} diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallestModulesAnalyzer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallestModulesAnalyzer.scala index c99177e69c..fc827978ee 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallestModulesAnalyzer.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/SmallestModulesAnalyzer.scala @@ -12,10 +12,9 @@ package org.scalajs.linker.frontend.modulesplitter -import scala.annotation.tailrec import scala.collection.mutable -import org.scalajs.ir.Names.{ClassName, ObjectClass} +import org.scalajs.ir.Names.ClassName import org.scalajs.linker.standard.ModuleSet.ModuleID /** Build smallest possible modules. @@ -33,146 +32,20 @@ private[modulesplitter] final class SmallestModulesAnalyzer extends ModuleAnalyz } } -private object SmallestModulesAnalyzer { - private final class Node(val className: ClassName, val index: Int) { - var lowlink: Int = index - var moduleIndex: Int = -1 - } +private[modulesplitter] object SmallestModulesAnalyzer { - private class Run(info: ModuleAnalyzer.DependencyInfo) - extends ModuleAnalyzer.Analysis { + private final class Run(info: ModuleAnalyzer.DependencyInfo) + extends StrongConnect(info) with ModuleAnalyzer.Analysis { - private[this] var nextIndex = 0 - private[this] val nodes = mutable.Map.empty[ClassName, Node] - private[this] val stack = mutable.ArrayBuffer.empty[Node] private[this] val moduleIndexToID = mutable.Map.empty[Int, ModuleID] - private[this] val toConnect = mutable.Queue.empty[ClassName] def moduleForClass(className: ClassName): Option[ModuleID] = - nodes.get(className).map(n => moduleIndexToID(n.moduleIndex)) - - def analyze(): Unit = { - info.publicModuleDependencies - .valuesIterator - .flatten - .filter(!nodes.contains(_)) - .foreach(strongconnect(_)) - - assert(stack.isEmpty) - - while (toConnect.nonEmpty) { - val clazz = toConnect.dequeue() - if (!nodes.contains(clazz)) - strongconnect(clazz) - } - } - - private def strongconnect(className: ClassName): Node = { - /* Tarjan's algorithm for strongly connected components. - * - * The intuition is as follows: We determine a single spanning tree using - * a DFS (recursive calls to `strongconnect`). - * - * Whenever we find a back-edge (i.e. an edge to a node already visited), - * we know that the current sub-branch (up to that node) is strongly - * connected. This is because it can be "cycled through" through the cycle - * we just discovered. - * - * A strongly connected component is identified by the lowest index node - * that is part of it. This makes it easy to propagate and merge - * components. - * - * More: - * https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm - */ - assert(!nodes.contains(className)) - - val node = new Node(className, nextIndex) - nextIndex += 1 - - nodes(className) = node - stack += node - - val classInfo = info.classDependencies(className) - - /* Dynamic dependencies do not affect the import graph: It is OK to have - * cyclic, dynamic dependencies (because we never generate top-level - * awaits). - * - * However, we need to make sure the dynamic dependency is actually put - * into a module. For this, we schedule it to be connected later (we - * cannot connect it immediately, otherwise we'd mess up the - * stack/spanning tree state). - */ - classInfo.dynamicDependencies - // avoid enqueuing things we've already reached anyways. - .filter(!nodes.contains(_)) - .foreach(toConnect.enqueue(_)) - - for (depName <- classInfo.staticDependencies) { - nodes.get(depName).fold { - // We have not visited this dependency. It is part of our spanning tree. - val depNode = strongconnect(depName) - node.lowlink = math.min(node.lowlink, depNode.lowlink) - } { depNode => - // We have already visited this node. - if (depNode.moduleIndex == -1) { - // This is a back link. - node.lowlink = math.min(node.lowlink, depNode.index) - } - } - } - - if (node.lowlink == node.index) { - // This node is the root node of a component/module. - val moduleIndex = node.index - - var name = node.className - - @tailrec - def pop(): Unit = { - val n = stack.remove(stack.size - 1) - n.moduleIndex = moduleIndex - - /* Take the lexicographically smallest name as a stable name of the - * module, with the exception of j.l.Object which identifies the root - * module. - * - * We do this, because it is simple and stable (i.e. does not depend - * on traversal order). - */ - if (name != ObjectClass) { - if (n.className == ObjectClass) - name = ObjectClass - else if (n.className.compareTo(name) < 0) - name = n.className - } - - if (n ne node) - pop() - } - - pop() - - /* Build a module ID that doesn't collide with others. - * - * We observe: - * - Class names are unique, so they never collide with each other. - * - Appending a dot ('.') to a class name results in an illegal class name. - * - * So we append dots until we hit a ModuleID not used by a public module. - * - * Note that this is stable, because it does not depend on the order we - * iterate over nodes. - */ - var moduleID = ModuleID(name.nameString) - while (info.publicModuleDependencies.contains(moduleID)) - moduleID = ModuleID(moduleID.id + ".") - - moduleIndexToID(moduleIndex) = moduleID - } + moduleIndex(className).map(moduleIndexToID) - node + protected def emitModule(moduleIndex: Int, classNames: List[ClassName]): Unit = { + val repr = ModuleIDs.representativeClass(classNames) + val id = ModuleIDs.forClassName(info.publicModuleDependencies.keySet, repr) + moduleIndexToID(moduleIndex) = id } } } diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/StrongConnect.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/StrongConnect.scala new file mode 100644 index 0000000000..8d41cfe5cc --- /dev/null +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/StrongConnect.scala @@ -0,0 +1,139 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.linker.frontend.modulesplitter + +import scala.annotation.tailrec +import scala.collection.mutable + +import org.scalajs.ir.Names.ClassName +import org.scalajs.linker.standard.ModuleSet.ModuleID + +private object StrongConnect { + private final class Node(val className: ClassName, val index: Int) { + var lowlink: Int = index + var moduleIndex: Int = -1 + } +} + +/** Analyzer to find strongly connected components. */ +private abstract class StrongConnect(info: ModuleAnalyzer.DependencyInfo) { + import StrongConnect.Node + + private[this] var nextIndex = 0 + private[this] val nodes = mutable.Map.empty[ClassName, Node] + private[this] val stack = mutable.ArrayBuffer.empty[Node] + private[this] val toConnect = mutable.Queue.empty[ClassName] + + final def analyze(): Unit = { + info.publicModuleDependencies + .valuesIterator + .flatten + .filter(!nodes.contains(_)) + .foreach(strongconnect(_)) + + assert(stack.isEmpty) + + while (toConnect.nonEmpty) { + val clazz = toConnect.dequeue() + if (!nodes.contains(clazz)) + strongconnect(clazz) + } + } + + protected final def moduleIndex(className: ClassName): Option[Int] = + nodes.get(className).map(_.moduleIndex) + + /** Extension point; called once for each strongly connected component (during analyze). */ + protected def emitModule(moduleIndex: Int, classNames: List[ClassName]): Unit + + private def strongconnect(className: ClassName): Node = { + /* Tarjan's algorithm for strongly connected components. + * + * The intuition is as follows: We determine a single spanning tree using + * a DFS (recursive calls to `strongconnect`). + * + * Whenever we find a back-edge (i.e. an edge to a node already visited), + * we know that the current sub-branch (up to that node) is strongly + * connected. This is because it can be "cycled through" through the cycle + * we just discovered. + * + * A strongly connected component is identified by the lowest index node + * that is part of it. This makes it easy to propagate and merge + * components. + * + * More: + * https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm + */ + assert(!nodes.contains(className)) + + val node = new Node(className, nextIndex) + nextIndex += 1 + + nodes(className) = node + stack += node + + val classInfo = info.classDependencies(className) + + /* Dynamic dependencies do not affect the import graph: It is OK to have + * cyclic, dynamic dependencies (because we never generate top-level + * awaits). + * + * However, we need to make sure the dynamic dependency is actually put + * into a module. For this, we schedule it to be connected later (we + * cannot connect it immediately, otherwise we'd mess up the + * stack/spanning tree state). + */ + classInfo.dynamicDependencies + // avoid enqueuing things we've already reached anyways. + .filter(!nodes.contains(_)) + .foreach(toConnect.enqueue(_)) + + for (depName <- classInfo.staticDependencies) { + nodes.get(depName).fold { + // We have not visited this dependency. It is part of our spanning tree. + val depNode = strongconnect(depName) + node.lowlink = math.min(node.lowlink, depNode.lowlink) + } { depNode => + // We have already visited this node. + if (depNode.moduleIndex == -1) { + // This is a back link. + node.lowlink = math.min(node.lowlink, depNode.index) + } + } + } + + if (node.lowlink == node.index) { + // This node is the root node of a component/module. + val moduleIndex = node.index + + val classNames = List.newBuilder[ClassName] + + @tailrec + def pop(): Unit = { + val n = stack.remove(stack.size - 1) + n.moduleIndex = moduleIndex + + classNames += n.className + + if (n ne node) + pop() + } + + pop() + + emitModule(moduleIndex, classNames.result()) + } + + node + } +} diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/Tagger.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/Tagger.scala new file mode 100644 index 0000000000..9212fa9f13 --- /dev/null +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/Tagger.scala @@ -0,0 +1,235 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.linker.frontend.modulesplitter + +import scala.annotation.tailrec + +import scala.collection.immutable +import scala.collection.mutable + +import java.nio.charset.StandardCharsets + +import org.scalajs.ir.Names.ClassName +import org.scalajs.ir.SHA1 +import org.scalajs.linker.standard.ModuleSet.ModuleID + + +/** Tagger groups classes into coarse modules. + * + * It is the primary mechanism for the FewestModulesAnalyzer but also used + * by the SmallModulesForAnalyzer. + * + * To group classes into modules appropriately, we want to know for + * each class, "how" it can be reached. In practice, this means we + * record the path from the original public module and every + * dynamic import hop we made. + * + * Of all these paths, we only care about the "simplest" ones. Or + * more formally, the minimum prefixes of all paths. For example, + * if a class is reachable by the following paths: + * + * - a -> b + * - a -> b -> c + * - d -> c + * - d + * + * We really only care about: + * + * - a -> b + * - d + * + * Because if we reach the class through a path that goes through + * `c`, it is necessarily already loaded. + * + * Once we have obtained this minimal set of paths, we use the last + * element of each path to determine the final module + * grouping. This is because these have an actual static dependency + * on the node in question. + * + * Merging these tags into a single `ModuleID` is delegated to the + * caller. + * + * == Class Exclusion == + * Classes can be excluded from the modules generated by Tagger. + * + * For excluded classes, the Tagger assumes that they are in a module + * provided by a different part of the overall analysis. + * + * The (transitive) dependencies of the class are nevertheless taken into + * account and tagged as appropriate. + */ +private class Tagger(infos: ModuleAnalyzer.DependencyInfo, + excludedClasses: scala.collection.Set[ClassName] = Set.empty) { + import Tagger._ + + private[this] val allPaths = mutable.Map.empty[ClassName, Paths] + + final def tagAll(internalModuleIDPrefix: String): scala.collection.Map[ClassName, ModuleID] = { + tagEntryPoints() + for { + (className, paths) <- allPaths + if !excludedClasses.contains(className) + } yield { + className -> paths.moduleID(internalModuleIDPrefix) + } + } + + private def tag(className: ClassName, pathRoot: ModuleID, pathSteps: List[ClassName]): Unit = { + val updated = allPaths + .getOrElseUpdate(className, new Paths) + .put(pathRoot, pathSteps) + + if (updated) { + val classInfo = infos.classDependencies(className) + classInfo + .staticDependencies + .foreach(staticEdge(_, pathRoot, pathSteps)) + + classInfo + .dynamicDependencies + .foreach(dynamicEdge(_, pathRoot, pathSteps)) + } + } + + private def staticEdge(className: ClassName, pathRoot: ModuleID, pathSteps: List[ClassName]): Unit = { + if (excludedClasses.contains(className)) + // Force a "dynamic edge" to the external module. + dynamicEdge(className, pathRoot, pathSteps) + else + tag(className, pathRoot, pathSteps) + } + + private def dynamicEdge(className: ClassName, pathRoot: ModuleID, pathSteps: List[ClassName]): Unit = + tag(className, pathRoot, pathSteps :+ className) + + private def tagEntryPoints(): Unit = { + for { + (moduleID, deps) <- infos.publicModuleDependencies + className <- deps + } { + staticEdge(className, moduleID, Nil) + } + } +} + +private object Tagger { + + /** "Interesting" paths that can lead to a given class. + * + * "Interesting" in this context means: + * - All direct paths from a public dependency. + * - All non-empty, mutually prefix-free paths of dynamic import hops. + */ + private final class Paths { + private val direct = mutable.Set.empty[ModuleID] + private val dynamic = mutable.Map.empty[ModuleID, DynamicPaths] + + def put(pathRoot: ModuleID, pathSteps: List[ClassName]): Boolean = { + if (pathSteps.isEmpty) { + direct.add(pathRoot) + } else { + dynamic + .getOrElseUpdate(pathRoot, new DynamicPaths) + .put(pathSteps) + } + } + + def moduleID(internalModuleIDPrefix: String): ModuleID = { + if (direct.size == 1 && dynamic.isEmpty) { + /* Class is only used by a single public module. Put it there. + * + * Note that we must not do this if there are any dynamic modules + * requiring this class. Otherwise, the dynamically loaded module + * will try to import the public module (but importing public modules is + * forbidden). + */ + direct.head + } else { + /* Class is used by multiple public modules and/or dynamic edges. + * Create a module ID grouping it with other classes that have the same + * dependees. + */ + val digestBuilder = new SHA1.DigestBuilder + + // Public modules using this. + for (id <- direct.toList.sortBy(_.id)) + digestBuilder.update(id.id.getBytes(StandardCharsets.UTF_8)) + + // Dynamic modules using this. + for (className <- dynamicEnds) + digestBuilder.updateUTF8String(className.encoded) + + // Build a hex string of the hash with the right prefix. + @inline def hexDigit(digit: Int): Char = + Character.forDigit(digit & 0x0f, 16) + + val id = new java.lang.StringBuilder(internalModuleIDPrefix) + + for (b <- digestBuilder.finalizeDigest()) { + id.append(hexDigit(b >> 4)) + id.append(hexDigit(b)) + } + + ModuleID(id.toString()) + } + } + + private def dynamicEnds: immutable.SortedSet[ClassName] = { + val builder = immutable.SortedSet.newBuilder[ClassName] + /* We ignore paths that originate in a module that imports this class + * directly: They are irrelevant for the final ID. + * + * However, they are important to ensure we do not attempt to import a + * public module (see the comment in moduleID); therefore, we only filter + * them here. + */ + for ((h, t) <- dynamic if !direct.contains(h)) + t.ends(builder) + builder.result() + } + } + + /** Set of shortest, mutually prefix-free paths of dynamic import hops */ + private final class DynamicPaths { + private val content = mutable.Map.empty[ClassName, DynamicPaths] + + @tailrec + def put(path: List[ClassName]): Boolean = { + val h :: t = path + + if (content.get(h).exists(_.content.isEmpty)) { + // shorter or equal path already exists. + false + } else if (t.isEmpty) { + // the path we put stops here, prune longer paths (if any). + content.put(h, new DynamicPaths) + true + } else { + // there are other paths, recurse. + content + .getOrElseUpdate(h, new DynamicPaths) + .put(t) + } + } + + /** Populates `builder` with the ends of all paths. */ + def ends(builder: mutable.Builder[ClassName, Set[ClassName]]): Unit = { + for ((h, t) <- content) { + if (t.content.isEmpty) + builder += h + else + t.ends(builder) + } + } + } +} diff --git a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala index b33238c994..883a76374b 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala @@ -183,7 +183,10 @@ class OptimizerTest { }) ) - for (moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers, TestIRRepo.fulllib)) yield { + for { + moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers, + stdlib = TestIRRepo.fulllib) + } yield { assertFalse(findClass(moduleSet, ClassClass).isDefined) } } @@ -403,7 +406,7 @@ class OptimizerTest { for { moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers, - linkerConfig = linkerConfig) + config = linkerConfig) } yield { assertFalse(findClass(moduleSet, "Thunk").isDefined) @@ -430,52 +433,4 @@ object OptimizerTest { private def findClass(moduleSet: ModuleSet, name: ClassName): Option[LinkedClass] = moduleSet.modules.flatMap(_.classDefs).find(_.className == name) - - private final class StoreModuleSetLinkerBackend( - originalBackend: LinkerBackend) - extends LinkerBackend { - - @volatile - private var _moduleSet: ModuleSet = _ - - val coreSpec: CoreSpec = originalBackend.coreSpec - - val symbolRequirements: SymbolRequirement = originalBackend.symbolRequirements - - override def injectedIRFiles: Seq[IRFile] = originalBackend.injectedIRFiles - - def emit(moduleSet: ModuleSet, output: OutputDirectory, logger: Logger)( - implicit ec: ExecutionContext): Future[Report] = { - _moduleSet = moduleSet - originalBackend.emit(moduleSet, output, logger) - } - - def moduleSet: ModuleSet = { - if (_moduleSet == null) - throw new IllegalStateException("Cannot access moduleSet before emit is called") - _moduleSet - } - } - - def linkToModuleSet(classDefs: Seq[ClassDef], - moduleInitializers: List[ModuleInitializer], - stdlib: Future[Seq[IRFile]] = TestIRRepo.minilib, - linkerConfig: StandardConfig = StandardConfig())( - implicit ec: ExecutionContext): Future[ModuleSet] = { - - val config = linkerConfig.withCheckIR(true) - val frontend = StandardLinkerFrontend(config) - val backend = new StoreModuleSetLinkerBackend(StandardLinkerBackend(config)) - val linker = StandardLinkerImpl(frontend, backend) - - val classDefsFiles = classDefs.map(MemClassDefIRFile(_)) - val output = MemOutputDirectory() - - stdlib.flatMap { stdLibFiles => - linker.link(stdLibFiles ++ classDefsFiles, moduleInitializers, - output, new ScalaConsoleLogger(Level.Error)) - }.map { _ => - backend.moduleSet - } - } } diff --git a/linker/shared/src/test/scala/org/scalajs/linker/SmallModulesForSplittingTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/SmallModulesForSplittingTest.scala new file mode 100644 index 0000000000..aff889e2e1 --- /dev/null +++ b/linker/shared/src/test/scala/org/scalajs/linker/SmallModulesForSplittingTest.scala @@ -0,0 +1,95 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.linker + +import scala.concurrent._ + +import org.junit.Test +import org.junit.Assert._ + +import org.scalajs.ir.ClassKind +import org.scalajs.ir.Names._ +import org.scalajs.ir.Trees._ +import org.scalajs.ir.Types._ + +import org.scalajs.junit.async._ + +import org.scalajs.linker.interface._ +import org.scalajs.linker.testutils.LinkingUtils._ +import org.scalajs.linker.testutils.TestIRBuilder._ + +class SmallModulesForSplittingTest { + import scala.concurrent.ExecutionContext.Implicits.global + + @Test + def splitsModules(): AsyncResult = await { + /* Test splitting in the degenerate case, where dependencies traverse the + * split boundary multiple times. + */ + val strClsType = ClassType(BoxedStringClass) + + val methodName = m("get", Nil, T) + + val SMF = EMF.withNamespace(MemberNamespace.PublicStatic) + + def methodHolder(name: ClassName, body: Tree) = { + classDef(name, + kind = ClassKind.Interface, + memberDefs = List( + MethodDef(SMF, methodName, NON, Nil, strClsType, Some(body))( + EOH.withNoinline(true), None) + )) + } + + def call(name: ClassName): Tree = + ApplyStatic(EAF, name, methodName, Nil)(strClsType) + + val helloWorldClass = "helloworld.HelloWorld$" + + val classDefs = Seq( + methodHolder("foo.A", str("Hello World")), + methodHolder("bar.B", call("foo.A")), + methodHolder("foo.C", call("bar.B")), + mainTestClassDef(consoleLog(call("foo.C"))) + ) + + val linkerConfig = StandardConfig() + .withModuleKind(ModuleKind.ESModule) + .withModuleSplitStyle(ModuleSplitStyle.SmallModulesFor(List("foo"))) + .withSourceMap(false) + + for { + moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers, + config = linkerConfig) + } yield { + def moduleClasses(id: String) = { + val module = moduleSet.modules.find(_.id.id == id).getOrElse { + val ids = moduleSet.modules.map(_.id.id).mkString(", ") + throw new AssertionError(s"couldn't find module with id: `$id`, got: [$ids]") + } + + module.classDefs.map(_.name.name) + } + + assertEquals(List[ClassName]("foo.A"), moduleClasses("foo.A")) + assertEquals(List[ClassName]("foo.C"), moduleClasses("foo.C")) + assertEquals(List(MainTestClassName), moduleClasses("main")) + + /* Expect two additional modules, one for each: + * - Scala.js core + * - bar.B + */ + assertEquals(5, moduleSet.modules.size) + } + } +} diff --git a/linker/shared/src/test/scala/org/scalajs/linker/testutils/LinkingUtils.scala b/linker/shared/src/test/scala/org/scalajs/linker/testutils/LinkingUtils.scala index 62d36b66a6..b108a9a69f 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/testutils/LinkingUtils.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/testutils/LinkingUtils.scala @@ -19,6 +19,7 @@ import org.scalajs.ir.Trees.ClassDef import org.scalajs.logging._ import org.scalajs.linker._ +import org.scalajs.linker.standard._ import org.scalajs.linker.interface._ object LinkingUtils { @@ -36,4 +37,51 @@ object LinkingUtils { output, new ScalaConsoleLogger(Level.Error)) } } + + private final class StoreModuleSetLinkerBackend(originalBackend: LinkerBackend) + extends LinkerBackend { + + @volatile + private var _moduleSet: ModuleSet = _ + + val coreSpec: CoreSpec = originalBackend.coreSpec + + val symbolRequirements: SymbolRequirement = originalBackend.symbolRequirements + + override def injectedIRFiles: Seq[IRFile] = originalBackend.injectedIRFiles + + def emit(moduleSet: ModuleSet, output: OutputDirectory, logger: Logger)( + implicit ec: ExecutionContext): Future[Report] = { + _moduleSet = moduleSet + originalBackend.emit(moduleSet, output, logger) + } + + def moduleSet: ModuleSet = { + if (_moduleSet == null) + throw new IllegalStateException("Cannot access moduleSet before emit is called") + _moduleSet + } + } + + def linkToModuleSet(classDefs: Seq[ClassDef], + moduleInitializers: List[ModuleInitializer], + config: StandardConfig = StandardConfig(), + stdlib: Future[Seq[IRFile]] = TestIRRepo.minilib)( + implicit ec: ExecutionContext): Future[ModuleSet] = { + + val cfg = config.withCheckIR(true) + val frontend = StandardLinkerFrontend(cfg) + val backend = new StoreModuleSetLinkerBackend(StandardLinkerBackend(cfg)) + val linker = StandardLinkerImpl(frontend, backend) + + val classDefsFiles = classDefs.map(MemClassDefIRFile(_)) + val output = MemOutputDirectory() + + stdlib.flatMap { stdLibFiles => + linker.link(stdLibFiles ++ classDefsFiles, moduleInitializers, + output, new ScalaConsoleLogger(Level.Error)) + }.map { _ => + backend.moduleSet + } + } } From 6619c4b84c25ab36efdc038be67dddbc56581101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Sun, 3 Apr 2022 23:09:19 +0200 Subject: [PATCH 20/20] Version 1.10.0. --- ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala b/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala index 512f04c890..14de811b5f 100644 --- a/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala +++ b/ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala @@ -17,7 +17,7 @@ import java.util.concurrent.ConcurrentHashMap import scala.util.matching.Regex object ScalaJSVersions extends VersionChecks( - current = "1.10.0-SNAPSHOT", + current = "1.10.0", binaryEmitted = "1.8" )