Skip to content

Conversation

@JackStouffer
Copy link
Contributor

@JackStouffer JackStouffer commented May 22, 2017

Following #5404, this PR optimizes the fromISOString constructor of TimeOfDay substantially.

Changes

  • Removed conversion to dstring as to no longer auto decodes
  • Used conversion to uint to auto check for bad characters, removing three extra loops
  • Replaces use of format with text for error messages, as format is slower due to string parsing when all you really need for this is concatenation

Results

$ ldc2 -O -release test.d && ./test
old	4 secs, 347 ms, 869 μs, and 3 hnsecs
new	929 ms, 331 μs, and 1 hnsec

$ dmd -release -inline -O test.d && ./test
old	9 secs, 175 ms, 614 μs, and 7 hnsecs
new	1 sec, 896 ms, 854 μs, and 5 hnsecs

Benchmark code

import std.stdio;
import std.algorithm;
import std.conv;
import std.ascii;
import std.range;
import std.traits;
import std.string;
import std.datetime;
import std.utf;

enum testCount = 20_000_000;

TimeOfDay fromISOString1(S)(in S isoString) @safe pure
    if (isSomeString!S)
{
    import std.algorithm.searching : all;
    import std.ascii : isDigit;
    import std.conv : to;
    import std.exception : enforce;
    import std.format : format;
    import std.string : strip;

    auto dstr = to!dstring(strip(isoString));

    enforce(dstr.length == 6, new DateTimeException(format("Invalid ISO String: %s", isoString)));

    auto hours = dstr[0 .. 2];
    auto minutes = dstr[2 .. 4];
    auto seconds = dstr[4 .. $];

    enforce(all!isDigit(hours), new DateTimeException(format("Invalid ISO String: %s", isoString)));
    enforce(all!isDigit(minutes), new DateTimeException(format("Invalid ISO String: %s", isoString)));
    enforce(all!isDigit(seconds), new DateTimeException(format("Invalid ISO String: %s", isoString)));

    return TimeOfDay(to!int(hours), to!int(minutes), to!int(seconds));
}

TimeOfDay fromISOString2(S)(in S isoString) @safe pure
    if (isSomeString!S)
{
    import std.conv : to, text, ConvException;
    import std.exception : enforce;
    import std.string : strip;

    int hours, minutes, seconds;
    auto str = strip(isoString);

    enforce!DateTimeException(str.length == 6, text("Invalid ISO String: ", isoString));

    try
    {
        // cast to int from uint is used because it checks for
        // non digits without extra loops
        hours = cast(int) to!uint(str[0 .. 2]);
        minutes = cast(int) to!uint(str[2 .. 4]);
        seconds = cast(int) to!uint(str[4 .. $]);
    }
    catch (ConvException)
    {
        throw new DateTimeException(text("Invalid ISO String: ", isoString));
    }

    return TimeOfDay(hours, minutes, seconds);
}

void main()
{
    auto result = to!Duration(benchmark!(() => fromISOString1("123033"))(testCount)[0]);
    auto result2 = to!Duration(benchmark!(() => fromISOString2("123033"))(testCount)[0]);

    writeln("old", "\t", result);
    writeln("new", "\t", result2);
}

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good to me, but I defer the merge to the "guardian" of std.datetime @jmdavis ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants