Skip to content

Commit

Permalink
Fix scala-native#2902: Avoid Array allocation in ieee754tostring impl…
Browse files Browse the repository at this point in the history
…ementations (scala-native#2917)

* Fix scala-native#2902: Avoid Array allocation in ieee754tostring implementations

* Address reviewer suggestions in PR scala-native#2917:
* add comment to describe RESULT_STRING_MAX_LENGTH magic numbers
* add private method _xxxToCharsNoCheck() (e.g. isNaN) to avoid redundant checks when calling xxxToString()
* mark xxxToString() methods as @deprecated (note: this line is commented, as it is currently leading to errors in tests)
* add new unit tests in RyuDoubleTest and RyuFloatTest (not asked by the reviewer)
* add unit test in StringBuilderTest
* renamed and reorganize tests in StringBufferTest

* Remove unnecessary @noinline annotations

* Remove methods RyuDouble.doubleToString()/RyuFloat.floatToString() and put simplified versions in RyuDoubleTest/RyuFloatTest

* PR scala-native#2902 improvements:
* re-implement Double.toString/Float.toString in order to use RyuDouble.doubleToChars()/RyuFloat.floatToChars()
* simplify doubleToString()/floatToString() example wrappers in unit tests
* Add a note to highlight `result.length - offset >= RESULT_STRING_MAX_LENGTH`
  • Loading branch information
david-bouyssie committed Nov 1, 2022
1 parent 396c163 commit 9446fe6
Show file tree
Hide file tree
Showing 11 changed files with 262 additions and 75 deletions.
34 changes: 34 additions & 0 deletions javalib/src/main/scala/java/lang/AbstractStringBuilder.scala
Expand Up @@ -5,7 +5,10 @@ import java.io.InvalidObjectException
import java.util.Arrays
import scala.util.control.Breaks._

import scala.scalanative.runtime.ieee754tostring.ryu._

protected abstract class AbstractStringBuilder private (unit: Unit) {

import AbstractStringBuilder._

protected var value: Array[Char] = _
Expand Down Expand Up @@ -108,7 +111,38 @@ protected abstract class AbstractStringBuilder private (unit: Unit) {
count += 1
}

// Optimization: use `RyuFloat.floatToChars()` instead of `floatToString()`
protected final def append0(f: scala.Float): Unit = {

// We first ensure that we have enough space in the backing Array (`value`)
this.ensureCapacity(this.count + RyuFloat.RESULT_STRING_MAX_LENGTH)

// Then we call `RyuFloat.floatToChars()`, which will append chars to `value`
this.count = RyuFloat.floatToChars(
f,
RyuRoundingMode.Conservative,
value,
this.count
)
}

// Optimization: use `RyuFloat.doubleToChars()` instead of `doubleToString()`
protected final def append0(d: scala.Double): Unit = {

// We first ensure that we have enough space in the backing Array (`value`)
this.ensureCapacity(this.count + RyuDouble.RESULT_STRING_MAX_LENGTH)

// Then we call `RyuFloat.doubleToChars()`, which will append chars to `value`
this.count = RyuDouble.doubleToChars(
d,
RyuRoundingMode.Conservative,
value,
this.count
)
}

protected final def append0(string: String): Unit = {

if (string == null) {
appendNull()
return
Expand Down
5 changes: 4 additions & 1 deletion javalib/src/main/scala/java/lang/Double.scala
Expand Up @@ -318,7 +318,10 @@ object Double {
}

@inline def toString(d: scala.Double): String = {
RyuDouble.doubleToString(d, RyuRoundingMode.Conservative)
val result = new scala.Array[Char](RyuDouble.RESULT_STRING_MAX_LENGTH)
val strLen =
RyuDouble.doubleToChars(d, RyuRoundingMode.Conservative, result, 0)
new _String(0, strLen, result).asInstanceOf[String]
}

@inline def valueOf(d: scala.Double): Double =
Expand Down
5 changes: 4 additions & 1 deletion javalib/src/main/scala/java/lang/Float.scala
Expand Up @@ -320,7 +320,10 @@ object Float {
}

def toString(f: scala.Float): String = {
RyuFloat.floatToString(f, RyuRoundingMode.Conservative)
val result = new scala.Array[Char](RyuFloat.RESULT_STRING_MAX_LENGTH)
val strLen =
RyuFloat.floatToChars(f, RyuRoundingMode.Conservative, result, 0)
new _String(0, strLen, result).asInstanceOf[String]
}

@inline def valueOf(s: String): Float =
Expand Down
12 changes: 8 additions & 4 deletions javalib/src/main/scala/java/lang/StringBuffer.scala
Expand Up @@ -43,11 +43,15 @@ final class StringBuffer
this
}

def append(d: scala.Double): StringBuffer =
append(Double.toString(d))
def append(f: scala.Float): StringBuffer = {
append0(f)
this
}

def append(f: scala.Float): StringBuffer =
append(Float.toString(f))
def append(d: scala.Double): StringBuffer = {
append0(d)
this
}

def append(i: scala.Int): StringBuffer =
append(Integer.toString(i))
Expand Down
4 changes: 2 additions & 2 deletions javalib/src/main/scala/java/lang/StringBuilder.scala
Expand Up @@ -50,12 +50,12 @@ final class StringBuilder
}

def append(f: scala.Float): StringBuilder = {
append0(Float.toString(f))
append0(f)
this
}

def append(d: scala.Double): StringBuilder = {
append0(Double.toString(d))
append0(d)
this
}

Expand Down
Expand Up @@ -35,10 +35,12 @@ package scala.scalanative
package runtime
package ieee754tostring.ryu

import RyuRoundingMode._

object RyuDouble {

// Scala/Java magic number 24 is derived from original RYU C code magic number 25 (which includes NUL terminator).
// See https://github.com/ulfjack/ryu/blob/6f85836b6389dce334692829d818cdedb28bfa00/ryu/d2s.c#L506
final val RESULT_STRING_MAX_LENGTH = 24

final val DOUBLE_MANTISSA_BITS = 52

final val DOUBLE_MANTISSA_MASK = (1L << DOUBLE_MANTISSA_BITS) - 1
Expand Down Expand Up @@ -694,26 +696,67 @@ object RyuDouble {

// format: on

@noinline def doubleToString(
@inline
private def copyLiteralToCharArray(
literal: String,
literalLength: Int,
result: scala.Array[Char],
offset: Int
): Int = {
literal.getChars(0, literalLength, result, offset)
offset + literalLength
}

// See: https://github.com/scala-native/scala-native/issues/2902
/** Low-level function executing the Ryu algorithm on `Double` value. This
* function allows destination passing style. This means that the result
* destination (`Array[Char]`) has to be passed as an argument. The goal is
* to avoid additional allocations when possible. Warnings: this function
* makes no verification of destination bounds (offset and length are assumed
* to be valid). The caller must thus ensure that `result.length - offset >=
* RESULT_STRING_MAX_LENGTH`.
*
* @param value
* the value to be converted
* @param roundingMode
* customization of Ryu rounding mode
* @param result
* the `Array[Char]` destination of the conversion result
* @param offset
* index in `Array[Char]` destination where new chars will start to be
* written
* @return
* new offset as: old offset + number of created chars (i.e. last modified
* index + 1)
*/
def doubleToChars(
value: Double,
roundingMode: RyuRoundingMode
): String = {
roundingMode: RyuRoundingMode,
result: scala.Array[Char],
offset: Int
): Int = {

// Handle all the trivial cases.
if (value.isNaN)
return copyLiteralToCharArray("NaN", 3, result, offset)
if (value == Double.PositiveInfinity)
return copyLiteralToCharArray("Infinity", 8, result, offset)
if (value == Double.NegativeInfinity)
return copyLiteralToCharArray("-Infinity", 9, result, offset)

// Step 1: Decode the floating point number, and unify normalized and
// subnormal cases.
// First, handle all the trivial cases.
if (value.isNaN) return "NaN"
if (value == Double.PositiveInfinity) return "Infinity"
if (value == Double.NegativeInfinity) return "-Infinity"
val bits = java.lang.Double.doubleToLongBits(value)
if (bits == 0) return "0.0"
if (bits == 0x8000000000000000L) return "-0.0"
if (bits == 0)
return copyLiteralToCharArray("0.0", 3, result, offset)
if (bits == 0x8000000000000000L)
return copyLiteralToCharArray("-0.0", 4, result, offset)

// Otherwise extract the mantissa and exponent bits and run the full
// algorithm.
// Otherwise extract the mantissa and exponent bits and run the full algorithm.
// Step 1: Decode the floating point number, and unify normalized and subnormal cases.
val ieeeExponent =
((bits >>> DOUBLE_MANTISSA_BITS) & DOUBLE_EXPONENT_MASK).toInt
val ieeeMantissa = bits & DOUBLE_MANTISSA_MASK

// By default, the correct mantissa starts with a 1, except for denormal numbers.
var e2 = 0
var m2 = 0L
if (ieeeExponent == 0) {
Expand All @@ -732,7 +775,7 @@ object RyuDouble {
val mv = 4 * m2
val mp = 4 * m2 + 2
val mmShift =
if (((m2 != (1L << DOUBLE_MANTISSA_BITS)) || (ieeeExponent <= 1))) 1
if ((m2 != (1L << DOUBLE_MANTISSA_BITS)) || (ieeeExponent <= 1)) 1
else 0
val mm = 4 * m2 - 1 - mmShift
e2 -= 2
Expand Down Expand Up @@ -786,21 +829,18 @@ object RyuDouble {
}
}

// Step 4: Find the shortest decimal representation in the interval of
// legal representations.
// Step 4: Find the shortest decimal representation in the interval of legal representations.
//
// We do some extra work here in order to follow Float/Double.toString
// semantics. In particular, that requires printing in scientific format
// if and only if the exponent is between -3 and 7, and it requires
// printing at least two decimal digits.
//
// Above, we moved the decimal dot all the way to the right, so now we
// need to count digits to
// figure out the correct exponent for scientific notation.
// Above, we moved the decimal dot all the way to the right, so now we need to count digits
// to figure out the correct exponent for scientific notation.
val vplength = decimalLength(dp)
var exp = e10 + vplength - 1
// Double.toString semantics requires using scientific notation if and
// only if outside this range.
// Double.toString semantics requires using scientific notation if and only if outside this range.
val scientificNotation = !((exp >= -3) && (exp < 7))
var removed = 0
var lastRemovedDigit = 0
Expand Down Expand Up @@ -868,8 +908,7 @@ object RyuDouble {

// Step 5: Print the decimal representation.
// We follow Double.toString semantics here.
val result = new scala.Array[Char](24)
var index = 0
var index = offset
if (sign) {
result(index) = '-'
index += 1
Expand All @@ -890,8 +929,7 @@ object RyuDouble {
index += 1
}

// Print 'E', the exponent sign, and the exponent, which has at most
// three digits.
// Print 'E', the exponent sign, and the exponent, which has at most three digits.
result(index) = 'E'
index += 1
if (exp < 0) {
Expand All @@ -911,7 +949,6 @@ object RyuDouble {
}
result(index) = ('0' + exp % 10).toChar
index += 1
new String(result, 0, index)
} else {
// Otherwise follow the Java spec for values in the interval [1E-3, 1E7).
if (exp < 0) {
Expand Down Expand Up @@ -959,8 +996,9 @@ object RyuDouble {
}
index += olength + 1
}
new String(result, 0, index)
}

index
}

private def pow5bits(e: Int): Int = ((e * 1217359) >>> 19) + 1
Expand Down

0 comments on commit 9446fe6

Please sign in to comment.