Removed auto-decoding from the floating point version of std.conv.parse by JackStouffer · Pull Request #5015 · dlang/phobos

JackStouffer · 2017-01-04T20:57:53Z

New version of #4674

13.5x faster! Code here.

# new
$ ldc2 -O5 -release test.d && ./test
Total time: 387 ms, 506 μs, and 8 hnsecs

# old
$ ldc2 -O5 -release test.d && ./test
Total time: 5 secs, 238 ms, 59 μs, and 4 hnsecs

JackStouffer · 2017-01-04T21:04:33Z

ping @andralex

andralex

Yah this would be a lot easier to follow if it continued to use p...

andralex · 2017-01-05T00:54:56Z

std/conv.d


-    char sign = 0;                       /* indicating +                 */
-    switch (p.front)
+    bool sign = false;


andralex · 2017-01-05T00:57:59Z

std/conv.d

-        p.popFront();
-        enforce(!p.empty, bailOut());
-        if (toLower(p.front) == 'i')
+        sign = true;


Hmmm... looks like both the previous code and the current code are parsing --5 as positive 5, right?

So I guess an enforce(!sign) is in order here. In fact sign should be a ternary: not seen, + seen, - seen.

andralex · 2017-01-05T01:27:20Z

std/conv.d

-        enforce(!p.empty, bailOut());
-        if (toLower(p.front) == 'i')
+        sign = true;
+        source.popFront();


(For simpler refactoring: continue using p and give the initial parameter a different name.)

andralex · 2017-01-05T01:28:20Z

std/conv.d

        {
+            static if (isNarrowString!Source)
+                p = source.assumeUTF;
+


no reason for vertical space here

andralex · 2017-01-05T01:28:27Z

std/conv.d

+            static if (isNarrowString!Source)
+                p = source.assumeUTF;
+
            return (sign) ? -0.0 : 0.0;


drop the parens

cc @wilzbach :)

andralex · 2017-01-05T01:29:11Z

std/conv.d

        }

-        isHex = p.front == 'x' || p.front == 'X';
+        isHex = source.front == 'x' || source.front == 'X';


source.front.among('x', 'X') if it's convenient

andralex · 2017-01-05T01:42:46Z

std/conv.d


        enforce(anydigits, bailOut());
-        enforce(!p.empty && (p.front == 'p' || p.front == 'P'),
+        enforce(!source.empty && (source.front == 'p' || source.front == 'P'),


among could be of use again

JackStouffer · 2017-01-05T05:52:29Z

Went back to p so the diff is now much smaller

JackStouffer · 2017-01-05T06:28:21Z

Auto-merge toggled on

jondegenhardt · 2017-01-06T17:51:22Z

Very nice! This should improve performance of conversions by std.conv.to from string to float or double, right? If that's the case, it might be worth including the std.conv.to benefit in the update notes. Might not be obvious.

micro-optimization in std.conv.parse

2b09695

JackStouffer mentioned this pull request Jan 4, 2017

Removed auto-decoding from std.conv.parse (part 2) #4674

Closed

andralex approved these changes Jan 5, 2017

View reviewed changes

Removed auto-decoding from the floating point version of std.conv.parse

a308446

JackStouffer force-pushed the parse-decoding3 branch from e5fef35 to a308446 Compare January 5, 2017 05:51

JackStouffer merged commit dae2040 into dlang:master Jan 5, 2017

JackStouffer deleted the parse-decoding3 branch May 22, 2017 17:34

PetarKirov mentioned this pull request Jun 27, 2017

Performance regressions without LTO in LDC 1.3.0-beta[1|2] ldc-developers/ldc#2168

Open

joakim-noah mentioned this pull request Jul 17, 2018

Extend std.conv.parse for strings to full ieeeQuadruple precision and other fixes #6633

Merged

joakim-noah added a commit to ldc-developers/phobos that referenced this pull request Aug 7, 2018

std.conv: Fix error message and doc comment that changed in dlang#5015.

400080c

Uh oh!

Comments

Conversation

JackStouffer commented Jan 4, 2017

Uh oh!

JackStouffer commented Jan 4, 2017

Uh oh!

andralex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JackStouffer commented Jan 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JackStouffer commented Jan 5, 2017

Uh oh!

jondegenhardt commented Jan 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JackStouffer commented Jan 5, 2017 •

edited

Loading