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

Pool or otherwise remove some buffers in StringAsPinnedUTF8 on NET6+ #2208

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stevenaw
Copy link

This PR targets .NET6+ but all of the targeted APIs are also available on lower runtimes if dependencies were to be taken on NuGet packages such as System.Buffers.

The PR avoids double-allocation of buffers and adds array pooling to the needed buffer to reduce memory usage to a consistent value independent of payload size while also improving a bit of the performance.

Benchmarks (using .NET8 runtime)
| Method   | N   | Mean     | Error     | StdDev   | Ratio | RatioSD | Gen0   | Allocated | Alloc Ratio |
|--------- |---- |---------:|----------:|---------:|------:|--------:|-------:|----------:|------------:|
| Original | 11  | 36.63 ns | 15.401 ns | 0.844 ns |  1.00 |    0.00 | 0.0166 |     104 B |        1.00 |
| Updated  | 11  | 34.86 ns |  1.437 ns | 0.079 ns |  0.95 |    0.02 | 0.0051 |      32 B |        0.31 |
|          |     |          |           |          |       |         |        |           |             |
| Original | 64  | 39.95 ns |  2.437 ns | 0.134 ns |  1.00 |    0.00 | 0.0331 |     208 B |        1.00 |
| Updated  | 64  | 41.82 ns | 14.281 ns | 0.783 ns |  1.05 |    0.02 | 0.0051 |      32 B |        0.15 |
|          |     |          |           |          |       |         |        |           |             |
| Original | 128 | 46.58 ns | 15.269 ns | 0.837 ns |  1.00 |    0.00 | 0.0535 |     336 B |        1.00 |
| Updated  | 128 | 42.75 ns | 10.538 ns | 0.578 ns |  0.92 |    0.02 | 0.0051 |      32 B |        0.10 |
        [MemoryDiagnoser]
    public class Utf8Pinner
    {
        // 11 = "Hello World" :)
        [Params(11, 64, 128)]
        public int N { get; set; }

        public string Payload { get; set; }

        [GlobalSetup]
        public void Setup()
        {
            Payload = new string('A', N);
        }


        [Benchmark(Baseline = true)]
        public IntPtr Original()
        {
            using var pinner = new OriginalUTF8Pinner(Payload);
            return pinner.Ptr;
        }

        [Benchmark]
        public IntPtr Updated()
        {
            using var pinner = new UpdatedUTF8Pinner(Payload);
            return pinner.Ptr;
        }

        public sealed class UpdatedUTF8Pinner : IDisposable
        {
            private readonly GCHandle gch;

#if NET6_0_OR_GREATER
            private readonly byte[] strBytesNulTerminated;

            public UpdatedUTF8Pinner(string str)
            {
                var size = Encoding.UTF8.GetMaxByteCount(str.Length);

                // Ask for one extra for the null byte
                strBytesNulTerminated = ArrayPool<byte>.Shared.Rent(size + 1);

                try
                {
                    Span<byte> slice = strBytesNulTerminated.AsSpan().Slice(0, size);
                    int bytesWritten = Encoding.UTF8.GetBytes(str, slice);

                    // 0-init the remainder
                    Array.Clear(strBytesNulTerminated, bytesWritten, strBytesNulTerminated.Length - bytesWritten);
                }
                catch
                {
                    // An exception above means the object never creates, potentially leaving a
                    // memory leak as there would be no object to Dispose() of later
                    ArrayPool<byte>.Shared.Return(strBytesNulTerminated);
                    throw;
                }

                this.gch = GCHandle.Alloc(strBytesNulTerminated, GCHandleType.Pinned);
            }

#else
            public UpdatedUTF8Pinner(string str)
            {
                byte[] strBytes = System.Text.UTF8Encoding.UTF8.GetBytes(str);
                byte[] strBytesNulTerminated = new byte[strBytes.Length + 1]; // initialized to all 0's.
                Array.Copy(strBytes, strBytesNulTerminated, strBytes.Length);
                this.gch = GCHandle.Alloc(strBytesNulTerminated, GCHandleType.Pinned);
            } 
#endif

            public IntPtr Ptr { get => this.gch.AddrOfPinnedObject(); }

            public void Dispose()
            {
                gch.Free();
#if NET6_0_OR_GREATER
                ArrayPool<byte>.Shared.Return(strBytesNulTerminated);
#endif
            }
        }

        public sealed class OriginalUTF8Pinner : IDisposable
        {
            private GCHandle gch;

            public OriginalUTF8Pinner(string str)
            {
                byte[] strBytes = System.Text.UTF8Encoding.UTF8.GetBytes(str);
                byte[] strBytesNulTerminated = new byte[strBytes.Length + 1]; // initialized to all 0's.
                Array.Copy(strBytes, strBytesNulTerminated, strBytes.Length);
                this.gch = GCHandle.Alloc(strBytesNulTerminated, GCHandleType.Pinned);
            }

            public IntPtr Ptr { get => this.gch.AddrOfPinnedObject(); }

            public void Dispose()
            {
                gch.Free();
            }
        }
    }
</details>

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.

None yet

1 participant