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

Rewrite TagList for .NET 8+ #104132

Merged
merged 1 commit into from
Jun 28, 2024
Merged

Rewrite TagList for .NET 8+ #104132

merged 1 commit into from
Jun 28, 2024

Conversation

stephentoub
Copy link
Member

Renamed the existing TagList.cs file to be TagList.netfx.cs, then copied it to a TagList.netcore.cs file and rewrote the guts of it to use [InlineArray] for the embedded key/value pairs.

Method Toolchain key value Mean Ratio Code Size
AddTagListToCounter1 \main\corerun.exe ? ? 6.973 ns 1.00 942 B
AddTagListToCounter1 \pr\corerun.exe ? ? 5.752 ns 0.82 465 B
AddTagListToCounter7 \main\corerun.exe ? ? 39.900 ns 1.00 1,201 B
AddTagListToCounter7 \pr\corerun.exe ? ? 14.767 ns 0.37 1,248 B
ForEach \main\corerun.exe ? ? 76.632 ns 1.00 588 B
ForEach \pr\corerun.exe ? ? 50.252 ns 0.66 420 B
RemoveAdd \main\corerun.exe ? ? 429.194 ns 1.00 1,339 B
RemoveAdd \pr\corerun.exe ? ? 151.181 ns 0.35 1,140 B
IndexOf \main\corerun.exe Name1 Val1 2.075 ns 1.00 2,010 B
IndexOf \pr\corerun.exe Name1 Val1 3.102 ns 1.49 622 B
IndexOf \main\corerun.exe Name7 Val7 20.122 ns 1.00 2,699 B
IndexOf \pr\corerun.exe Name7 Val7 18.327 ns 0.91 415 B
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Diagnostics.Metrics;
using System.Diagnostics;

BenchmarkSwitcher.FromAssembly(typeof(Tests).Assembly).Run(args);

[DisassemblyDiagnoser]
[HideColumns("Job", "Error", "StdDev", "Median", "RatioSD")]
public class Tests
{
    private Counter<long> _counter;
    private Meter _meter;
    private TagList _list = new TagList(
    [
        KeyValuePair.Create("Name1", (object?)"Val1"),
        KeyValuePair.Create("Name2", (object?)"Val2"),
        KeyValuePair.Create("Name3", (object?)"Val3"),
        KeyValuePair.Create("Name4", (object?)"Val4"),
        KeyValuePair.Create("Name5", (object?)"Val5"),
        KeyValuePair.Create("Name6", (object?)"Val6"),
        KeyValuePair.Create("Name7", (object?)"Val7"),
    ]);
    private TagList _other = new TagList();

    [GlobalSetup]
    public void Setup()
    {
        this._meter = new Meter("TestMeter");
        this._counter = this._meter.CreateCounter<long>("counter");
    }

    [GlobalCleanup]
    public void Cleanup() => this._meter.Dispose();

    [Benchmark]
    public void AddTagListToCounter1()
    {
        this._counter?.Add(100, new TagList
        {
            { "Name1", "Val1" },
        });
    }

    [Benchmark]
    public void AddTagListToCounter7()
    {
        this._counter?.Add(100, new TagList
        {
            { "Name1", "Val1" },
            { "Name2", "Val2" },
            { "Name3", "Val3" },
            { "Name4", "Val4" },
            { "Name5", "Val5" },
            { "Name6", "Val6" },
            { "Name7", "Val7" },
        });
    }

    [Benchmark]
    public int ForEach()
    {
        int count = 0;
        foreach (KeyValuePair<string, object?> item in _list)
        {
            count += item.Value is not null ? 1 : 0;
        }
        return count;
    }

    [Benchmark]
    public void RemoveAdd()
    {
        while (_list.Count > 0)
        {
            _other.Add(_list[^1]);
            _list.RemoveAt(_list.Count - 1);
        }

        while (_other.Count > 0)
        {
            _list.Add(_other[^1]);
            _other.RemoveAt(_other.Count - 1);
        }
    }

    [Benchmark]
    [Arguments("Name1", "Val1")]
    [Arguments("Name7", "Val7")]
    public int IndexOf(string key, object? value) => _list.IndexOf(KeyValuePair.Create(key, value));
}

Renamed the existing TagList.cs file to be TagList.netfx.cs, then copied it to a TagList.netcore.cs file and rewrote the guts of it to use [InlineArray] for the embedded key/value pairs.
Copy link
Contributor

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

@tarekgh
Copy link
Member

tarekgh commented Jun 28, 2024

Any idea why first IndexOf regressed with Val1? would be the same for like 3 or 4 tags too? I don't think IndexOf is used much in TagList, I'm just curious about the result.

@tarekgh
Copy link
Member

tarekgh commented Jun 28, 2024

CC @noahfalk @stevejgordon @cijothomas

Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks @stephentoub.

@stephentoub
Copy link
Member Author

Any idea why first IndexOf regressed with Val1? would be the same for like 3 or 4 tags too? I don't think IndexOf is used much in TagList, I'm just curious about the result.

I don't think the IndexOf results are particularly meaningful one way or the other. It's basically just +/- a nanosecond:

Method Toolchain key value Mean Ratio Code Size
IndexOf \main\corerun.exe Name1 Val1 2.185 ns 1.00 2,010 B
IndexOf \pr\corerun.exe Name1 Val1 2.734 ns 1.25 622 B
IndexOf \main\corerun.exe Name2 Val2 6.195 ns 1.00 2,195 B
IndexOf \pr\corerun.exe Name2 Val2 6.189 ns 1.00 415 B
IndexOf \main\corerun.exe Name3 Val3 8.727 ns 1.00 2,464 B
IndexOf \pr\corerun.exe Name3 Val3 8.702 ns 1.00 415 B
IndexOf \main\corerun.exe Name4 Val4 9.615 ns 1.00 2,634 B
IndexOf \pr\corerun.exe Name4 Val4 10.907 ns 1.13 415 B
IndexOf \main\corerun.exe Name5 Val5 15.273 ns 1.00 2,782 B
IndexOf \pr\corerun.exe Name5 Val5 14.714 ns 0.96 415 B
IndexOf \main\corerun.exe Name6 Val6 17.673 ns 1.00 2,610 B
IndexOf \pr\corerun.exe Name6 Val6 16.578 ns 0.94 415 B
IndexOf \main\corerun.exe Name7 Val7 20.479 ns 1.00 2,699 B
IndexOf \pr\corerun.exe Name7 Val7 18.508 ns 0.90 415 B

@stephentoub
Copy link
Member Author

/ba-g failures are known

@stephentoub stephentoub merged commit c796529 into dotnet:main Jun 28, 2024
80 of 83 checks passed
@stephentoub stephentoub deleted the taglistarray branch June 28, 2024 17:24
/// Using more than eight tags will cause allocating memory to store the tags.
/// Public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.
/// </remarks>
[StructLayout(LayoutKind.Sequential)]
Copy link
Contributor

Choose a reason for hiding this comment

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

@stephentoub I'm missing the reason why this is necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not. It just came along as part of the copy from from the original code and I neglected to delete it.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants