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

[JackStouffer]

Benchmark code here: https://gist.github.com/JackStouffer/af53d497532a864f56a5b53cb35cd438

Gives about a 40% speed increase:

# old
$ ldc2 -O5 -release -boundscheck=off test.d && ./test
Total time: 5 secs, 266 ms, 64 μs, and 3 hnsecs

# new
$ ldc2 -O5 -release -boundscheck=off test.d && ./test
Total time: 3 secs, 295 ms, 663 μs, and 7 hnsecs

[JackStouffer]

Ping @9il

[codecov-io]

Current coverage is 88.74% (diff: 100%)

Merging #4674 into master will decrease coverage by <.01%

@@             master      #4674   diff @@
==========================================
  Files           121        121          
  Lines         74037      74043     +6   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          65705      65709     +4   
- Misses         8332       8334     +2   
  Partials          0          0          

Powered by Codecov. Last update 4a634ed...da24815

[9il]

LGTM

[9il]

Please add small msg to change log

[JackStouffer]

@9il Done

[9il]

@JackStouffer please add test (to the PR description) with global __gshared char[] integer number and pragma(inline, false) for parse functions. 4x-5x is large number, so, we need to be sure that LDC optimisation does not break the test.

[wilzbach]

@JackStouffer please add test with global __gshared char[] integer number and pragma(inline, false) for parse functions. 4x-5x is large number, so, we need to be sure that LDC optimisation does not brake the test.

IIRC @klickverbot pointed out that a good way to prevent LDC optimizations is to use separate files & annotate the tested methods with extern.

I have a small repo for benchmark scripts which are separately compiled and then linked, maybe that helps you?

[9il]

btw, pragma(inline, false) works correctly only in the recent LDC alpha, or master

[JackStouffer]

Rebased.

Will add extra test when I get time.

[JackStouffer]

@9il I updated the benchmark code here to your requests: #4597

[JackStouffer]

Ping @9il

[andralex]

This doesn't look right. Most code should stay the same. The only change is at the top level, where decodable strings should be replaced with byChar or .representation.

[andralex]

Where is part 1? I hope it doesn't make similar changes... :)

[JackStouffer]

@andralex Here's the problem, as outlined here

The reason why this is so messy is parse takes its range by ref, so byCodeUnit cannot be used without using refRange, which requires using &r, which is unsafe. So the only solution left is to do it manually with a bunch of static ifs.

I tried putting the refRange.byCodeUnit in a @trusted block, but that resulted in a speed pessimization (I'm guessing due to the inability to inline at that point).

[andralex]

@JackStouffer then just work on a temporary and in the end assign the reference from it. It's a classic.

[JackStouffer]

I don't understand. Operate on a temporary and reassign it how?

If I make a copy of p inside of parse and then use byCodeUnit on that, the copy of p is copied again by byCodeUnit and that is advanced and not the copy of p. So I would still have to manually popFront the copy in order to reassign it to p.

[JackStouffer]

Ping @andralex @9il

[9il]

change log should be updated. ping @wilzbach . You wanted to change current change log system

[9il]

@JackStouffer it is save to reassing ubyte array to a string after integer parsing. Please implement Andrei's suggestions

[JackStouffer]

I will get back to this eventually. I'm a bit swamped with school and work right now, so maybe around Thanksgiving.

[andralex]

@JackStouffer thanks, ping me when ready.

[JackStouffer]

Closed in favor of #5015