Hi,

I just took a look into commonly used functionality of Phobos.
Such as getting the string representation of a enum.

the following code:

import std.conv;
enum ET
{
  One,
  Two
}

static assert(to!string(ET.One) == "One");

takes about 220 milliseconds to compile.
creating a 7.5k object file

Using my -vcg-ast it becomes visible that it expands to ~17000 lines of template-instantiations.
explaining both the compilation time and the size.

Compiling the following code:

string enumToString(E)(E v)
{
    static assert(is(E == enum), "emumToString is only meant for enums");
    mixin ({
    string result = "final switch(v) {\n";
    foreach(m;[__traits(allMembers, E)])
    {
        result ~= "\tcase E." ~ m ~ " :\n"
            ~ "\t\treturn \"" ~ m ~ "\";\n"
            ~ "\tbreak;\n";
    }
    return result ~ "}";
    } ());
}

private enum ET
{
  One,
  Two
}

static assert (enumToString(ET.One) == "One");

takes about 4 milliseconds to compile.
creating a 4.8k object file.

Granted this version will result in undefined behavior if you pass something like (cast(ET) 3) to it.
But the 55x increase in compilation speed is well worth it :)

On Thursday, 18 May 2017 at 22:31:47 UTC, Stefan Koch wrote:
> Hi,
>
> I just took a look into commonly used functionality of Phobos.
> Such as getting the string representation of a enum.
>
> [...]
Using -vcg-ast we see that it expands to ~50 lines.

On Thursday, 18 May 2017 at 22:31:47 UTC, Stefan Koch wrote:
> Hi,
>
> I just took a look into commonly used functionality of Phobos.
> Such as getting the string representation of a enum.
>
> [...]

Nice, thank you. I dream of a guide to compile time optimization in D. :)

On 05/19/2017 12:31 AM, Stefan Koch wrote:
> string enumToString(E)(E v)
> {
>      static assert(is(E == enum), "emumToString is only meant for enums");
>      mixin ({
>      string result = "final switch(v) {\n";
>      foreach(m;[__traits(allMembers, E)])
>      {
>          result ~= "\tcase E." ~ m ~ " :\n"
>              ~ "\t\treturn \"" ~ m ~ "\";\n"
>              ~ "\tbreak;\n";
>      }
>      return result ~ "}";
>      } ());
> }

I'm sure that can be de-uglified a fair bit without hurting performance.

1) "final switch(v) {" and the closing brace can be moved out of the string. This should be completely free.

2) No need for `break` after `return`. Also free.

3) With a static foreach over `__traits(allMembers, E)` you can get rid of the function literal. Doesn't seem to affect performance much if at all.

So far:

----
string enumToString(E)(E v)
{
    static assert(is(E == enum),
        "emumToString is only meant for enums");
    final switch (v)
    {
        foreach(m; __traits(allMembers, E))
        {
            mixin("case E." ~ m ~ ": return \"" ~ m ~ "\";");
        }
    }
}
----

4) If EnumMembers is an option, you can get rid of the string mixin altogether:

----
string enumToString(E)(E v)
{
    import std.meta: AliasSeq;
    import std.traits: EnumMembers;
    static assert(is(E == enum),
        "emumToString is only meant for enums");
    alias memberNames = AliasSeq!(__traits(allMembers, E));
    final switch(v)
    {
        foreach(i, m; EnumMembers!E)
        {
            case m: return memberNames[i];
        }
    }
}
----

That takes a bit longer. May just be the time it takes to parse the std.* modules. Object size stays the same.

On Thursday, 18 May 2017 at 22:31:47 UTC, Stefan Koch wrote:

> Granted this version will result in undefined behavior if you pass something like (cast(ET) 3) to it.
> But the 55x increase in compilation speed is well worth it :)

This code will replicate to!string behavior perfectly but will only take 30 milliseconds to compile:

string enumToString(E)(E v)
{
    static assert(is(E == enum), "emumToString is only meant for enums");
    mixin({
    string result = "switch(v) {\n";
    foreach(m;[__traits(allMembers, E)])
    {
        result ~= "\tcase E." ~ m ~ " :\n"
            ~ "\t\treturn \"" ~ m ~ "\";\n";
    }
    result ~= "\tdefault: break;\n";
    result ~= "}\n";
    enum headLength = E.stringof.length + "cast()".length;
    result ~= `
    enum headLength = ` ~ headLength.stringof ~ `;
    uint val = v;
    char[` ~ (headLength + 10).stringof ~ `] res = "cast(` ~ E.stringof ~ `)";
    uint log10Val = (val < 10) ? 0 : (val < 100) ? 1 : (val < 1000) ? 2 : (val < 10000) ? 3
        : (val < 100000) ? 4 : (val < 1000000) ? 5 : (val < 10000000) ? 6
        : (val < 100000000) ? 7 : (val < 1000000000) ? 8 : 9;
    foreach(i;0 .. log10Val + 1)
    {
        res[headLength + log10Val - i] = cast(char) ('0' + (val % 10));
        val /= 10;
    }

    return res[0 .. headLength + log10Val + 1].idup;
`;
   return result;
    } ());
}

On Thu, May 18, 2017 at 11:42:25PM +0000, Stefan Koch via Digitalmars-d wrote:
> On Thursday, 18 May 2017 at 22:31:47 UTC, Stefan Koch wrote:
> 
> > Granted this version will result in undefined behavior if you pass
> > something like (cast(ET) 3) to it.
> > But the 55x increase in compilation speed is well worth it :)
> 
> This code will replicate to!string behavior perfectly but will only take 30 milliseconds to compile:
> 
> string enumToString(E)(E v)
> {
[...snip awesome stuff...]

Where's the PR? ;-)


T

-- 
Windows: the ultimate triumph of marketing over technology. -- Adrian von Bidder

On Thursday, 18 May 2017 at 23:15:46 UTC, ag0aep6g wrote:
> On 05/19/2017 12:31 AM, Stefan Koch wrote:
>> string enumToString(E)(E v)
>> {
>>      static assert(is(E == enum), "emumToString is only meant for enums");
>>      mixin ({
>>      string result = "final switch(v) {\n";
>>      foreach(m;[__traits(allMembers, E)])
>>      {
>>          result ~= "\tcase E." ~ m ~ " :\n"
>>              ~ "\t\treturn \"" ~ m ~ "\";\n"
>>              ~ "\tbreak;\n";
>>      }
>>      return result ~ "}";
>>      } ());
>> }
>
> I'm sure that can be de-uglified a fair bit without hurting performance.
>
> 1) "final switch(v) {" and the closing brace can be moved out of the string. This should be completely free.
>
> 2) No need for `break` after `return`. Also free.
>
> 3) With a static foreach over `__traits(allMembers, E)` you can get rid of the function literal. Doesn't seem to affect performance much if at all.
>
> So far:
>
> ----
> string enumToString(E)(E v)
> {
>     static assert(is(E == enum),
>         "emumToString is only meant for enums");
>     final switch (v)
>     {
>         foreach(m; __traits(allMembers, E))
>         {
>             mixin("case E." ~ m ~ ": return \"" ~ m ~ "\";");
>         }
>     }
> }
> ----
>
> 4) If EnumMembers is an option, you can get rid of the string mixin altogether:
>
> ----
> string enumToString(E)(E v)
> {
>     import std.meta: AliasSeq;
>     import std.traits: EnumMembers;
>     static assert(is(E == enum),
>         "emumToString is only meant for enums");
>     alias memberNames = AliasSeq!(__traits(allMembers, E));
>     final switch(v)
>     {
>         foreach(i, m; EnumMembers!E)
>         {
>             case m: return memberNames[i];
>         }
>     }
> }
> ----
>
> That takes a bit longer. May just be the time it takes to parse the std.* modules. Object size stays the same.

Nice work beatifying the implementation.

Although AliasSeq and EnumMembers are unnecessary.
I incorporated your idea into the following version:

string enumToString(E)(E v)
{
    static assert(is(E == enum),
        "emumToString is only meant for enums");
    switch(v)
    {
        foreach(m; __traits(allMembers, E))
        {
            case mixin("E." ~ m) : return m;
        }

        default :
        {
            string result = "cast(" ~ E.stringof ~ ")";
            uint val = v;
            enum headLength = E.stringof.length + "cast()".length;
            uint log10Val = (val < 10) ? 0 : (val < 100) ? 1 : (val < 1000) ? 2 :
                (val < 10000) ? 3 : (val < 100000) ? 4 : (val < 1000000) ? 5 :
                (val < 10000000) ? 6 : (val < 100000000) ? 7 : (val < 1000000000) ? 8 : 9;
            result.length += log10Val + 1;
            for(uint i;i != log10Val + 1;i++)
            {
                cast(char)result[headLength + log10Val - i] = cast(char) ('0' + (val % 10));
                val /= 10;
            }

            return cast(string) result;
        }
    }
}

On Friday, 19 May 2017 at 00:14:05 UTC, Stefan Koch wrote:

> string enumToString(E)(E v)
> {
>     static assert(is(E == enum),
>         "emumToString is only meant for enums");

Why that assert? We can check it at compiletime. Doesn't this cry for a constraint? I would use asserts only ever for stuff that's only known at runtime.

string enumToString(E)(E v) if(is(E == enum))
{
   ...
}

On Friday, 19 May 2017 at 17:34:28 UTC, Dominikus Dittes Scherkl wrote:
> On Friday, 19 May 2017 at 00:14:05 UTC, Stefan Koch wrote:
>
>> string enumToString(E)(E v)
>> {
>>     static assert(is(E == enum),
>>         "emumToString is only meant for enums");
>
> Why that assert? We can check it at compiletime. Doesn't this cry for a constraint? I would use asserts only ever for stuff that's only known at runtime.
>
> string enumToString(E)(E v) if(is(E == enum))
> {
>    ...
> }

the static assert tells what's going on.
It it does result in a simple overload not found.

On Friday, 19 May 2017 at 17:47:42 UTC, Stefan Koch wrote:
> On Friday, 19 May 2017 at 17:34:28 UTC, Dominikus Dittes Scherkl wrote:
>> On Friday, 19 May 2017 at 00:14:05 UTC, Stefan Koch wrote:
>>
>>> string enumToString(E)(E v)
>>> {
>>>     static assert(is(E == enum),
>>>         "emumToString is only meant for enums");
>>
>> Why that assert? We can check it at compiletime. Doesn't this cry for a constraint? I would use asserts only ever for stuff that's only known at runtime.
>>
>> string enumToString(E)(E v) if(is(E == enum))
>> {
>>    ...
>> }
>
> the static assert tells what's going on.
> It it does result in a simple overload not found.

Hm. Maybe in this case it's ok, because enum is pretty much all that can be expected as argument to "enumToString". But normally I would calling not using a constraint "stealing overload possibilities", because it would not be possible to overload the same function for a different type if you use this kind of assert.
And the error message is not really better.