Skip to content

Conversation

@JackStouffer
Copy link
Contributor

Removed explicit auto-decoding and simplified/speed up format checking.

Previous PRs: #5550 #5404 #5413

Results:

$ ldc2 -O -release test.d && ./test
old	6 secs, 425 ms, 378 μs, and 3 hnsecs
new	809 ms, 503 μs, and 6 hnsecs

$ dmd -O -inline -release test.d && ./test
old	12 secs, 946 ms, 437 μs, and 4 hnsecs
new	2 secs, 325 ms, 651 μs, and 9 hnsecs

Code

import std.stdio;
import std.algorithm;
import std.conv;
import std.range;
import std.traits;
import std.datetime.date;
import std.datetime.stopwatch;

enum testCount = 20_000_000;

Date fromISOExtString1(S)(in S isoExtString) @safe pure
        if (isSomeString!(S))
    {
        import std.algorithm.searching : all, startsWith;
        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(isoExtString));

        enforce(dstr.length >= 10, new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));

        auto day = dstr[$-2 .. $];
        auto month = dstr[$-5 .. $-3];
        auto year = dstr[0 .. $-6];

        enforce(dstr[$-3] == '-', new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));
        enforce(dstr[$-6] == '-', new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));
        enforce(all!isDigit(day),
                new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));
        enforce(all!isDigit(month),
                new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));

        if (year.length > 4)
        {
            enforce(year.startsWith('-', '+'),
                    new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));
            enforce(all!isDigit(year[1..$]),
                    new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));
        }
        else
            enforce(all!isDigit(year),
                    new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString)));

        return Date(to!short(year), to!ubyte(month), to!ubyte(day));
    }

Date fromISOExtString2(S)(in S isoExtString) @safe pure
if (isSomeString!(S))
{
    import std.algorithm.searching : startsWith;
    import std.conv : to, ConvException;
    import std.format : format;
    import std.string : strip;

    auto str = strip(isoExtString);
    short year;
    ubyte month, day;

    if (str.length < 10 || str[$-3] != '-' || str[$-6] != '-')
        throw new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString));

    auto yearStr = str[0 .. $-6];
    if ((yearStr.length > 4 && !yearStr.startsWith('-', '+')) ||
        (yearStr.length <= 4 && yearStr.startsWith('-', '+')))
        throw new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString));

    try
    {
        day = to!ubyte(str[$-2 .. $]);
        month = to!ubyte(str[$-5 .. $-3]);
        year = to!short(yearStr);
    }
    catch (ConvException)
    {
        throw new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString));
    }

    return Date(year, month, day);
}

void main()
{
    auto result = to!Duration(benchmark!(() => fromISOExtString1("2010-07-04"))(testCount)[0]);
    auto result2 = to!Duration(benchmark!(() => fromISOExtString2("2010-07-04"))(testCount)[0]);

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

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @JackStouffer!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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.

To save other reviewers time looking. to (or to be exact the underlying parse) does error checking itself, see

auto signAtBegining = yearStr.startsWith('-', '+');
if ((yearStr.length > 4 && !signAtBegining) ||
(yearStr.length <= 4 && signAtBegining))
throw new DateTimeException(format("Invalid ISO Extended String: %s", isoExtString));
Copy link
Member

Choose a reason for hiding this comment

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

I hate to nitpick style, but would you please use braces if the condition is more than one line - either that or put the condition on a single line instead of breaking it up. IMHO, it's a readability problem if there are no braces when the condition is multiple lines.

Copy link
Member

Choose a reason for hiding this comment

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

Also, aside from the optimizer being smart about things here (which it may be), I would expect it to be faster if you broke this up so that it didn't check the length twice given that it's the same check in reverse.

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