Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: introduce source generator for enum code generation #585

Closed
wants to merge 33 commits into from

Conversation

Meir017
Copy link
Contributor

@Meir017 Meir017 commented Aug 11, 2024

closes #568

viewing the generated files
image

@Meir017
Copy link
Contributor Author

Meir017 commented Aug 11, 2024

example generated enum-utils class for RespAclCategories

// <auto-generated>
// This code was generated by the EnumsSourceGenerator source generator.
// </auto-generated>
using System;
using System.ComponentModel;
using System.Numerics;
using Garnet.server;

namespace Garnet.common;

/// <summary>
/// Utility methods for enums.
/// </summary>
public static partial class EnumUtils
{
    /// <summary>
    /// Tries to parse the enum value from the description.
    /// </summary>
    /// <param name="description">Enum description.</param>
    /// <param name="result">Enum value.</param>
    /// <returns>True if successful.</returns>
    public static bool TryParseRespAclCategoriesFromDescription(string description, out RespAclCategories result)
    {
        result = default;
        switch (description)
        {
            case "admin":
                result = RespAclCategories.Admin;
                return true;
            case "bitmap":
                result = RespAclCategories.Bitmap;
                return true;
            case "blocking":
                result = RespAclCategories.Blocking;
                return true;
            case "connection":
                result = RespAclCategories.Connection;
                return true;
            case "dangerous":
                result = RespAclCategories.Dangerous;
                return true;
            case "geo":
                result = RespAclCategories.Geo;
                return true;
            case "hash":
                result = RespAclCategories.Hash;
                return true;
            case "hyperloglog":
                result = RespAclCategories.HyperLogLog;
                return true;
            case "fast":
                result = RespAclCategories.Fast;
                return true;
            case "keyspace":
                result = RespAclCategories.KeySpace;
                return true;
            case "list":
                result = RespAclCategories.List;
                return true;
            case "pubsub":
                result = RespAclCategories.PubSub;
                return true;
            case "read":
                result = RespAclCategories.Read;
                return true;
            case "scripting":
                result = RespAclCategories.Scripting;
                return true;
            case "set":
                result = RespAclCategories.Set;
                return true;
            case "sortedset":
                result = RespAclCategories.SortedSet;
                return true;
            case "slow":
                result = RespAclCategories.Slow;
                return true;
            case "stream":
                result = RespAclCategories.Stream;
                return true;
            case "string":
                result = RespAclCategories.String;
                return true;
            case "transaction":
                result = RespAclCategories.Transaction;
                return true;
            case "write":
                result = RespAclCategories.Write;
                return true;
            case "garnet":
                result = RespAclCategories.Garnet;
                return true;
            case "custom":
                result = RespAclCategories.Custom;
                return true;
            case "all":
                result = RespAclCategories.All;
                return true;
        }

        return false;
    }



    /// <summary>
    /// Gets the descriptions of the set flags. Assumes the enum is a flags enum.
    /// If no description exists, returns the ToString() value of the input value.
    /// </summary>
    /// <param name="value">Enum value.</param>
    /// <returns>Array of descriptions.</returns>
    public static string[] GetRespAclCategoriesDescriptions(RespAclCategories value)
    {
        if (value is RespAclCategories.None) return ["None"];
        if (value is RespAclCategories.All) return ["all"];
        var setFlags = BitOperations.PopCount((ulong)value);
        if (setFlags == 1)
        {
            return value switch
            {
                RespAclCategories.Admin => ["admin"],
                RespAclCategories.Bitmap => ["bitmap"],
                RespAclCategories.Blocking => ["blocking"],
                RespAclCategories.Connection => ["connection"],
                RespAclCategories.Dangerous => ["dangerous"],
                RespAclCategories.Geo => ["geo"],
                RespAclCategories.Hash => ["hash"],
                RespAclCategories.HyperLogLog => ["hyperloglog"],
                RespAclCategories.Fast => ["fast"],
                RespAclCategories.KeySpace => ["keyspace"],
                RespAclCategories.List => ["list"],
                RespAclCategories.PubSub => ["pubsub"],
                RespAclCategories.Read => ["read"],
                RespAclCategories.Scripting => ["scripting"],
                RespAclCategories.Set => ["set"],
                RespAclCategories.SortedSet => ["sortedset"],
                RespAclCategories.Slow => ["slow"],
                RespAclCategories.Stream => ["stream"],
                RespAclCategories.String => ["string"],
                RespAclCategories.Transaction => ["transaction"],
                RespAclCategories.Write => ["write"],
                RespAclCategories.Garnet => ["garnet"],
                RespAclCategories.Custom => ["custom"],
                _ => [value.ToString()],
            };
        }

        var descriptions = new string[setFlags];
        var index = 0;
        if ((value & RespAclCategories.Admin) != 0) descriptions[index++] = "admin";
        if ((value & RespAclCategories.Bitmap) != 0) descriptions[index++] = "bitmap";
        if ((value & RespAclCategories.Blocking) != 0) descriptions[index++] = "blocking";
        if ((value & RespAclCategories.Connection) != 0) descriptions[index++] = "connection";
        if ((value & RespAclCategories.Dangerous) != 0) descriptions[index++] = "dangerous";
        if ((value & RespAclCategories.Geo) != 0) descriptions[index++] = "geo";
        if ((value & RespAclCategories.Hash) != 0) descriptions[index++] = "hash";
        if ((value & RespAclCategories.HyperLogLog) != 0) descriptions[index++] = "hyperloglog";
        if ((value & RespAclCategories.Fast) != 0) descriptions[index++] = "fast";
        if ((value & RespAclCategories.KeySpace) != 0) descriptions[index++] = "keyspace";
        if ((value & RespAclCategories.List) != 0) descriptions[index++] = "list";
        if ((value & RespAclCategories.PubSub) != 0) descriptions[index++] = "pubsub";
        if ((value & RespAclCategories.Read) != 0) descriptions[index++] = "read";
        if ((value & RespAclCategories.Scripting) != 0) descriptions[index++] = "scripting";
        if ((value & RespAclCategories.Set) != 0) descriptions[index++] = "set";
        if ((value & RespAclCategories.SortedSet) != 0) descriptions[index++] = "sortedset";
        if ((value & RespAclCategories.Slow) != 0) descriptions[index++] = "slow";
        if ((value & RespAclCategories.Stream) != 0) descriptions[index++] = "stream";
        if ((value & RespAclCategories.String) != 0) descriptions[index++] = "string";
        if ((value & RespAclCategories.Transaction) != 0) descriptions[index++] = "transaction";
        if ((value & RespAclCategories.Write) != 0) descriptions[index++] = "write";
        if ((value & RespAclCategories.Garnet) != 0) descriptions[index++] = "garnet";
        if ((value & RespAclCategories.Custom) != 0) descriptions[index++] = "custom";

        return descriptions;
    }

}

libs/gen/Garnet.gen.csproj Outdated Show resolved Hide resolved
@Meir017 Meir017 marked this pull request as ready for review August 12, 2024 05:15
@Meir017
Copy link
Contributor Author

Meir017 commented Aug 12, 2024

I just ran a small benchmark to compare:


BenchmarkDotNet v0.14.0, Windows 10 (10.0.19045.4651/22H2/2022Update)
Intel Core i7-6700HQ CPU 2.60GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
.NET SDK 8.0.303
  [Host]     : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2


Method Mean Error StdDev Gen0 Allocated
GetEnumDescriptions_SourceGenFlag1 10.541 ns 0.2041 ns 0.1809 ns 0.0102 32 B
GetEnumDescriptions_GarnetFlag1 177.819 ns 1.5695 ns 1.3106 ns 0.0713 224 B
GetEnumDescriptions_SourceGenFlag2 6.413 ns 0.1210 ns 0.1073 ns 0.0102 32 B
GetEnumDescriptions_GarnetFlag2 181.967 ns 1.9685 ns 1.7450 ns 0.0713 224 B
GetEnumDescriptions_SourceGenFlag3 10.860 ns 0.0698 ns 0.0545 ns 0.0127 40 B
GetEnumDescriptions_GarnetFlag3 321.769 ns 6.1681 ns 5.7696 ns 0.1349 424 B
GetEnumDescriptions_SourceGenFlag4 12.912 ns 0.2870 ns 0.2684 ns 0.0179 56 B
GetEnumDescriptions_GarnetFlag4 524.886 ns 5.7487 ns 5.0961 ns 0.2470 776 B
GetEnumDescriptions_SourceGenFlag5 5.994 ns 0.0429 ns 0.0358 ns 0.0102 32 B
TryParseEnumFromDescription_GarnetFlag1 50.095 ns 1.4312 ns 3.9419 ns 0.0102 32 B
TryParseEnumFromDescription_SourceGenFlag1 2.481 ns 0.0325 ns 0.0288 ns - -
TryParseEnumFromDescription_GarnetFlag2 43.657 ns 0.6137 ns 0.5740 ns 0.0102 32 B
TryParseEnumFromDescription_SourceGenFlag2 1.801 ns 0.0694 ns 0.0902 ns - -
TryParseEnumFromDescription_GarnetFlag3 44.333 ns 0.4689 ns 0.3916 ns 0.0102 32 B
TryParseEnumFromDescription_SourceGenFlag3 1.707 ns 0.0293 ns 0.0260 ns - -
TryParseEnumFromDescription_GarnetFlag4 53.670 ns 0.6426 ns 0.7142 ns 0.0102 32 B
TryParseEnumFromDescription_SourceGenFlag4 1.592 ns 0.0332 ns 0.0311 ns - -
TryParseEnumFromDescription_GarnetFlag5 41.455 ns 0.4447 ns 0.3714 ns 0.0102 32 B
TryParseEnumFromDescription_SourceGenFlag5 1.866 ns 0.0247 ns 0.0219 ns - -

with the following code:

[MemoryDiagnoser]
public class Benchmarks
{
    private readonly RespAclCategories flag1 = RespAclCategories.Garnet;
    private readonly RespAclCategories flag2 = RespAclCategories.Read;
    private readonly RespAclCategories flag3 = RespAclCategories.Read | RespAclCategories.Write;
    private readonly RespAclCategories flag4 = RespAclCategories.Read | RespAclCategories.Write | RespAclCategories.SortedSet | RespAclCategories.Transaction;
    private readonly RespAclCategories flag5 = RespAclCategories.All;

    [Benchmark]
    public string[] GetEnumDescriptions_SourceGenFlag1() => EnumUtils.GetRespAclCategoriesDescriptions(flag1);

    [Benchmark]
    public string[] GetEnumDescriptions_GarnetFlag1() => GarnetEnumUtils.GetEnumDescriptions(flag1);

    [Benchmark]
    public string[] GetEnumDescriptions_SourceGenFlag2() => EnumUtils.GetRespAclCategoriesDescriptions(flag2);

    [Benchmark]
    public string[] GetEnumDescriptions_GarnetFlag2() => GarnetEnumUtils.GetEnumDescriptions(flag2);

    [Benchmark]
    public string[] GetEnumDescriptions_SourceGenFlag3() => EnumUtils.GetRespAclCategoriesDescriptions(flag3);

    [Benchmark]
    public string[] GetEnumDescriptions_GarnetFlag3() => GarnetEnumUtils.GetEnumDescriptions(flag3);

    [Benchmark]
    public string[] GetEnumDescriptions_SourceGenFlag4() => EnumUtils.GetRespAclCategoriesDescriptions(flag4);

    [Benchmark]
    public string[] GetEnumDescriptions_GarnetFlag4() => GarnetEnumUtils.GetEnumDescriptions(flag4);

    [Benchmark]
    public string[] GetEnumDescriptions_SourceGenFlag5() => EnumUtils.GetRespAclCategoriesDescriptions(flag5);

    [Benchmark]
    public bool TryParseEnumFromDescription_GarnetFlag1() => GarnetEnumUtils.TryParseEnumFromDescription("Garnet", out RespAclCategories garnetParsedFlag1);

    [Benchmark]
    public bool TryParseEnumFromDescription_SourceGenFlag1() => EnumUtils.TryParseRespAclCategoriesFromDescription("Garnet", out RespAclCategories sourceGenParsedFlag1);

    [Benchmark]
    public bool TryParseEnumFromDescription_GarnetFlag2() => GarnetEnumUtils.TryParseEnumFromDescription("Read", out RespAclCategories garnetParsedFlag2);

    [Benchmark]
    public bool TryParseEnumFromDescription_SourceGenFlag2() => EnumUtils.TryParseRespAclCategoriesFromDescription("Read", out RespAclCategories sourceGenParsedFlag2);

    [Benchmark]
    public bool TryParseEnumFromDescription_GarnetFlag3() => GarnetEnumUtils.TryParseEnumFromDescription("Read, Write", out RespAclCategories garnetParsedFlag3);

    [Benchmark]
    public bool TryParseEnumFromDescription_SourceGenFlag3() => EnumUtils.TryParseRespAclCategoriesFromDescription("Read, Write", out RespAclCategories sourceGenParsedFlag3);

    [Benchmark]
    public bool TryParseEnumFromDescription_GarnetFlag4() => GarnetEnumUtils.TryParseEnumFromDescription("Read, Write, SortedSet, Transaction", out RespAclCategories garnetParsedFlag4);

    [Benchmark]
    public bool TryParseEnumFromDescription_SourceGenFlag4() => EnumUtils.TryParseRespAclCategoriesFromDescription("Read, Write, SortedSet, Transaction", out RespAclCategories sourceGenParsedFlag4);

    [Benchmark]
    public bool TryParseEnumFromDescription_GarnetFlag5() => GarnetEnumUtils.TryParseEnumFromDescription("All", out RespAclCategories garnetParsedFlag5);

    [Benchmark]
    public bool TryParseEnumFromDescription_SourceGenFlag5() => EnumUtils.TryParseRespAclCategoriesFromDescription("All", out RespAclCategories sourceGenParsedFlag5);
}

Copy link
Contributor

@PaulusParssinen PaulusParssinen left a comment

Choose a reason for hiding this comment

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

Great job with the work of making the source generator incremental, as they're very hard to get right (I could link dozens of instances where the most experienced .NET developers get them wrong too).

Some little suggestions left to fix couple issues in the pipeline caching (very, very common pitfalls), but otherwise, lgtm!

The ISourceGenerator interface should have never been made public, as it is confusing everyone now 😅

libs/gen/EnumsSourceGenerator.cs Outdated Show resolved Hide resolved
libs/gen/EnumsSourceGenerator.cs Outdated Show resolved Hide resolved
libs/gen/EnumsSourceGenerator.cs Outdated Show resolved Hide resolved
@TalZaccai TalZaccai self-requested a review August 13, 2024 20:24
@TalZaccai
Copy link
Contributor

Code generally looks good to me, however when we pull this locally we're having issues specifically in VS. I wasn't able to build Garnet.server (but dotnet build runs fine). This is the warning I'm seeing in the build output:
warning CS8032: An instance of analyzer Garnet.EnumsSourceGenerator cannot be created from D:\Git\GarnetOSS\libs\gen\bin\Debug\net8.0\Garnet.gen.dll : Could not load file or assembly 'System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified..
I asked another dev in the team to check and he said that VS won't even load the solution properly.
Any ideas on why we're seeing these issues here?

@PaulusParssinen
Copy link
Contributor

I think the source generator should target NS 2.0, not .NET 8.

@Meir017
Copy link
Contributor Author

Meir017 commented Aug 16, 2024

I think the source generator should target NS 2.0, not .NET 8.

This could explain why using the dotnet cli and VS doesn't.
I'll test using VS and migrate to netstandard2

@badrishc
Copy link
Contributor

Source generators in my experience create challenges with debugging and stability across platforms. We have to consider whether this will help us in critical paths. Adding it for non-critical regions of the code might bring more challenges than benefits. Also, we want to stick to net8.0 for the codebase for now. Adding ns2.0 might be overkill just for this capability. Just my 2 cents.

@Tornhoof
Copy link

Tornhoof commented Aug 17, 2024

Adding ns2.0 might be overkill just for this capability.

@badrishc fwiw, only the actual srcgen project needs to be netstandard 2.0, so VS can load it.

The question here is more, is build time srcgen the right approach? How often do these enums change? Is something like a t4 template maybe better?
( t4 templates suffer other problems, mostly bad tooling in . NET 5+, as VS still runs t4 in netfx)

Note: for larger enums (100+ values) the performance might get worse than dictionary for certain switch/case codepaths. Especially things like string->enum.

@badrishc
Copy link
Contributor

Makes sense. At the end of the day, the open question is (1) will this work seamlessly on all platforms; (2) are enums used in this way anywhere critical in Garnet, where this optimization will have a meaningful benefit.

@badrishc
Copy link
Contributor

I think the source generator should target NS 2.0, not .NET 8.

This could explain why using the dotnet cli and VS doesn't. I'll test using VS and migrate to netstandard2

was this change pushed?

@Meir017
Copy link
Contributor Author

Meir017 commented Aug 25, 2024

pushed now, working in VS

Recording.2024-08-25.152854.mp4

Makes sense. At the end of the day, the open question is
(1) will this work seamlessly on all platforms;

Source generators work on any environment (including AOT) as they are just part of the compilation phase and output plain txt files that can be included in the compilation of the projects (in our case .cs files)

(2) are enums used in this way anywhere critical in Garnet, where this optimization will have a meaningful benefit.

I think we could and should be able using source-generator to replace all of the usages of Enum. which aren't very performant (see https://andrewlock.net/netescapades-enumgenerators-a-source-generator-for-enum-performance/ ).

I this specific case when converting a flags-enum to a list of strings we are not reducing allocations (by not calling .Split on the ToString)

@badrishc
Copy link
Contributor

badrishc commented Dec 5, 2024

Closing as we have addressed this for the existing enums without using source generators, for now. Thank you!

@badrishc badrishc closed this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace EnumUtils with a source generator
5 participants