Skip to content

Commit 0f33c69

Browse files
aardvark179aardvark179
committed
[GR-17457] Reimplement Float#to_s using java.text.DecimalFormats.
PullRequest: truffleruby/3290
2 parents e85be2a + 4fa66e4 commit 0f33c69

File tree

4 files changed

+101
-39
lines changed

4 files changed

+101
-39
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Compatibility:
1818

1919
Performance:
2020

21+
* Reimplement `Float#to_s` for better performance (#1584, @aardvark179).
2122

2223
Changes:
2324

‎src/main/java/org/truffleruby/core/numeric/FloatNodes.java

Lines changed: 92 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.oracle.truffle.api.nodes.ExplodeLoop;
1919
import com.oracle.truffle.api.profiles.BranchProfile;
2020
import com.oracle.truffle.api.profiles.ConditionProfile;
21+
2122
import org.truffleruby.SuppressFBWarnings;
2223
import org.truffleruby.builtins.CoreMethod;
2324
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
@@ -29,14 +30,20 @@
2930
import org.truffleruby.core.encoding.Encodings;
3031
import org.truffleruby.core.numeric.FloatNodesFactory.ModNodeFactory;
3132
import org.truffleruby.core.rope.CodeRange;
33+
import org.truffleruby.core.rope.Rope;
34+
import org.truffleruby.core.rope.RopeOperations;
3235
import org.truffleruby.core.string.RubyString;
3336
import org.truffleruby.core.string.StringNodes;
3437
import org.truffleruby.core.string.StringUtils;
38+
import org.truffleruby.core.thread.RubyThread;
39+
import org.truffleruby.language.NotProvided;
3540
import org.truffleruby.language.RubyDynamicObject;
3641
import org.truffleruby.language.Visibility;
3742
import org.truffleruby.language.control.RaiseException;
3843
import org.truffleruby.language.dispatch.DispatchNode;
3944

45+
import java.text.DecimalFormat;
46+
import java.text.DecimalFormatSymbols;
4047
import java.util.Locale;
4148

4249
@CoreModule(value = "Float", isClass = true)
@@ -826,62 +833,109 @@ protected double toF(double value) {
826833
}
827834

828835
@CoreMethod(names = { "to_s", "inspect" })
836+
@ImportStatic(Double.class)
829837
public abstract static class ToSNode extends CoreMethodArrayArgumentsNode {
830838

831839
@Child private StringNodes.MakeStringNode makeStringNode = StringNodes.MakeStringNode.create();
832840

833-
@TruffleBoundary
834-
@Specialization
835-
protected RubyString toS(double value) {
836-
/* Ruby has complex custom formatting logic for floats. Our logic meets the specs but we suspect it's
837-
* possibly still not entirely correct. JRuby seems to be correct, but their logic is tied up in their
838-
* printf implementation. Also see our FormatFloatNode, which I suspect is also deficient or
839-
* under-tested. */
841+
/* Ruby has complex custom formatting logic for floats. Our logic meets the specs but we suspect it's possibly
842+
* still not entirely correct. JRuby seems to be correct, but their logic is tied up in their printf
843+
* implementation. Also see our FormatFloatNode, which I suspect is also deficient or under-tested. */
840844

841-
if (Double.isInfinite(value) || Double.isNaN(value)) {
842-
return makeStringNode.executeMake(Double.toString(value), Encodings.US_ASCII, CodeRange.CR_7BIT);
843-
}
845+
@Specialization(guards = "value == POSITIVE_INFINITY")
846+
protected RubyString toSPositiveInfinity(double value,
847+
@Cached("specialValueRope(POSITIVE_INFINITY)") Rope cachedRope) {
848+
return makeStringNode.executeMake(cachedRope, Encodings.US_ASCII, NotProvided.INSTANCE);
849+
}
844850

845-
String str = StringUtils.format(Locale.ENGLISH, "%.17g", value);
851+
@Specialization(guards = "value == NEGATIVE_INFINITY")
852+
protected RubyString toSNegativeInfinity(double value,
853+
@Cached("specialValueRope(NEGATIVE_INFINITY)") Rope cachedRope) {
854+
return makeStringNode.executeMake(cachedRope, Encodings.US_ASCII, NotProvided.INSTANCE);
855+
}
846856

847-
// If no dot, add one to show it's a floating point number
848-
if (str.indexOf('.') == -1) {
849-
assert str.indexOf('e') == -1;
850-
str += ".0";
851-
}
857+
@Specialization(guards = "isNaN(value)")
858+
protected RubyString toSNaN(double value,
859+
@Cached("specialValueRope(value)") Rope cachedRope) {
860+
return makeStringNode.executeMake(cachedRope, Encodings.US_ASCII, NotProvided.INSTANCE);
861+
}
852862

853-
final int dot = str.indexOf('.');
854-
assert dot != -1;
863+
@Specialization(guards = "hasNoExp(value)")
864+
protected RubyString toSNoExp(double value) {
865+
return makeStringNode.executeMake(makeRopeNoExp(value, getLanguage().getCurrentThread()),
866+
Encodings.US_ASCII, CodeRange.CR_7BIT);
867+
}
855868

856-
final int e = str.indexOf('e');
857-
final boolean hasE = e != -1;
869+
@Specialization(guards = "hasLargeExp(value)")
870+
protected RubyString toSLargeExp(double value) {
871+
return makeStringNode.executeMake(makeRopeLargeExp(value, getLanguage().getCurrentThread()),
872+
Encodings.US_ASCII, CodeRange.CR_7BIT);
873+
}
858874

859-
// Remove trailing zeroes, but keep at least one after the dot
860-
final int start = hasE ? e : str.length();
861-
int i = start - 1; // last digit we keep, inclusive
862-
while (i > dot + 1 && str.charAt(i) == '0') {
863-
i--;
864-
}
875+
@Specialization(guards = "hasSmallExp(value)")
876+
protected RubyString toSSmallExp(double value) {
877+
return makeStringNode.executeMake(makeRopeSmallExp(value, getLanguage().getCurrentThread()),
878+
Encodings.US_ASCII, CodeRange.CR_7BIT);
879+
}
880+
881+
@TruffleBoundary
882+
private String makeRopeNoExp(double value, RubyThread thread) {
883+
return getNoExpFormat(thread).format(value);
884+
}
865885

866-
String formatted = str.substring(0, i + 1) + str.substring(start);
886+
@TruffleBoundary
887+
private String makeRopeSmallExp(double value, RubyThread thread) {
888+
return getSmallExpFormat(thread).format(value);
889+
}
890+
891+
@TruffleBoundary
892+
private String makeRopeLargeExp(double value, RubyThread thread) {
893+
return getLargeExpFormat(thread).format(value);
894+
}
867895

868-
int wholeDigits = 0;
869-
int n = 0;
896+
protected static boolean hasNoExp(double value) {
897+
double abs = Math.abs(value);
898+
return abs == 0.0 || ((abs >= 0.0001) && (abs < 1_000_000_000_000_000.0));
899+
}
870900

871-
if (formatted.charAt(0) == '-') {
872-
n++;
873-
}
901+
protected static boolean hasLargeExp(double value) {
902+
double abs = Math.abs(value);
903+
return Double.isFinite(abs) && (abs >= 1_000_000_000_000_000.0);
904+
}
874905

875-
while (formatted.charAt(n) != '.') {
876-
wholeDigits++;
877-
n++;
906+
protected static boolean hasSmallExp(double value) {
907+
double abs = Math.abs(value);
908+
return (abs < 0.0001) && (abs != 0.0);
909+
}
910+
911+
protected static Rope specialValueRope(double value) {
912+
return RopeOperations.encodeAscii(Double.toString(value), Encodings.US_ASCII.jcoding);
913+
}
914+
915+
private DecimalFormat getNoExpFormat(RubyThread thread) {
916+
if (thread.noExpFormat == null) {
917+
final DecimalFormatSymbols noExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
918+
thread.noExpFormat = new DecimalFormat("0.0################", noExpSymbols);
878919
}
920+
return thread.noExpFormat;
921+
}
879922

880-
if (wholeDigits >= 16) {
881-
formatted = StringUtils.format(Locale.ENGLISH, "%.1e", value);
923+
private DecimalFormat getSmallExpFormat(RubyThread thread) {
924+
if (thread.smallExpFormat == null) {
925+
final DecimalFormatSymbols smallExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
926+
smallExpSymbols.setExponentSeparator("e");
927+
thread.smallExpFormat = new DecimalFormat("0.0################E00", smallExpSymbols);
882928
}
929+
return thread.smallExpFormat;
930+
}
883931

884-
return makeStringNode.executeMake(formatted, Encodings.US_ASCII, CodeRange.CR_7BIT);
932+
private DecimalFormat getLargeExpFormat(RubyThread thread) {
933+
if (thread.largeExpFormat == null) {
934+
final DecimalFormatSymbols largeExpSymbols = new DecimalFormatSymbols(Locale.ENGLISH);
935+
largeExpSymbols.setExponentSeparator("e+");
936+
thread.largeExpFormat = new DecimalFormat("0.0################E00", largeExpSymbols);
937+
}
938+
return thread.largeExpFormat;
885939
}
886940

887941
}

‎src/main/java/org/truffleruby/core/rope/RopeOperations.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public static LeafRope encodeAscii(String value, Encoding encoding) {
142142

143143
/** Prefer this to {@code getBytes(StandardCharsets.US_ASCII)} */
144144
public static byte[] encodeAsciiBytes(String value) {
145-
assert StringOperations.isAsciiOnly(value);
145+
assert StringOperations.isAsciiOnly(value) : "String contained non ascii characters \"" + value + "\"";
146146

147147
final byte[] bytes = new byte[value.length()];
148148

‎src/main/java/org/truffleruby/core/thread/RubyThread.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.truffleruby.core.thread;
1111

1212
import java.lang.invoke.VarHandle;
13+
import java.text.DecimalFormat;
1314
import java.util.ArrayList;
1415
import java.util.List;
1516
import java.util.Set;
@@ -66,6 +67,12 @@ public final class RubyThread extends RubyDynamicObject implements ObjectGraphNo
6667
public String sourceLocation;
6768
Object name = Nil.INSTANCE;
6869

70+
// Decimal formats are not thread safe, so we'll create them on the thread as we need them.
71+
72+
public DecimalFormat noExpFormat;
73+
public DecimalFormat smallExpFormat;
74+
public DecimalFormat largeExpFormat;
75+
6976
public RubyThread(
7077
RubyClass rubyClass,
7178
Shape shape,

0 commit comments

Comments
 (0)