Small performance increase for std.conv.text/wtext/dtext

[JackStouffer]

Saves about 5 - 7 hnsecs.

[DmitryOlshansky]

if args.length == 1 this will be slower I guess

[JackStouffer]

@DmitryOlshansky Fixed

[DmitryOlshansky]

Saves about 5 - 7 hnsecs.

How do you measure it?

[JackStouffer]

@DmitryOlshansky

import std.stdio;
import std.datetime;
import std.conv;

enum testCount = 1_000_000;

S textImpl(S, U...)(U args)
{
    static if (U.length == 0)
    {
        return null;
    }
    else
    {
        auto result = to!S(args[0]);
        foreach (arg; args[1 .. $])
            result ~= to!S(arg);
        return result;
    }
}

S textImpl2(S, U...)(U args)
{
    static if (U.length == 0)
    {
        return null;
    }
    else
    {
        import std.array : appender;

        if (__ctfe)
        {
            auto result = to!S(args[0]);
            foreach (arg; args[1 .. $])
                result ~= to!S(arg);
            return result;
        }
        else
        {
            auto app = appender!S();
            app.reserve(U.length * S.sizeof);

            foreach (arg; args)
                app.put(to!S(arg));
            return app.data;
        }
    }
}

void main()
{
    auto result = to!Duration(benchmark!(() => textImpl!dstring(42, ' ', 1.5, ": xyz", 333, "Another test"))(testCount)[0] / testCount);
    auto result2 = to!Duration(benchmark!(() => textImpl2!dstring(42, ' ', 1.5, ": xyz", 333, "Another test"))(testCount)[0] / testCount);

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

[JackStouffer]

I really don't see why the auto tester is failing.

[aG0aep6G]

Why S.sizeof? As far as I see, it's just an arbitrary number, determined by rule of thumb, and using S.sizeof for this suggests a relation where there is none.

[aG0aep6G]

Are you sure it's Appender that fails CTFE, and not to!S?

[aG0aep6G]

I really don't see why the auto tester is failing.

Compiler bug. Filed it here: https://issues.dlang.org/show_bug.cgi?id=15992

[JackStouffer]

Why S.sizeof?

|
|

Are you sure it's Appender that fails CTFE, and not to!S?

Yeah, not sure what I was thinking there. This never worked at CTFE.

Still creating one string per argument. How about using std.format.formatValue to format directly into the appender?

It's not @safe, and therefore it breaks a bunch of test in std.datetime. Also, I don't know if it's a good idea to make this unsafe in general.

[aG0aep6G]

It's not @safe, and therefore it breaks a bunch of test in std.datetime. Also, I don't know if it's a good idea to make this unsafe in general.

I see. Yeah, no, let's keep things @safe.

[9il]

appender should work in ctfe thought

[JackStouffer]

@aG0aep6G @9il fixed

[dlang-bot]

Fix	Bugzilla	Description
✓	15995	std.conv.text and friends can be made faster with std.array.appender

[JackStouffer]

Can someone add the "blocked" tag to this?

[schveiguy]

One heuristic optimization -- until an argument produces some actual string output, there's no need to fire up an appender. Essentially, you have the single-argument case, no need to copy the string that was just generated and throw that string away. Not sure if it helps or hurts the performance.

[schveiguy]

See my comment here: #4460 (comment)

[JackStouffer]

Can someone kick the auto-tester?

[9il]

app.formatValue(fmt, arg); should be used instead.

[9il]

You can add trusted, but please use formatValue instead

[JackStouffer]

You can add trusted

What if one of the arguments has a memory unsafe toString?

[JackStouffer]

Ready to go

[9il]

You can add trusted
What if one of the arguments has a memory unsafe toString?

Implementations should not be aware of what kind of toString is used, if they have both.

[9il]

Is current implementation always @safe?

[JackStouffer]

Is current implementation always @safe?

Depends on the implementation of to used for each of the arguments.

[JackStouffer]

formatValue is also impure, so that's a no go

[JackStouffer]

Ping @schveiguy @DmitryOlshansky @9il

[DmitryOlshansky]

LGTM

[schveiguy]

Auto-merge toggled on

[schveiguy]

I'd like to have seen an attempt to avoid appender until necessary, but OK.