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

[c#] Bond.IO.Safe.OutputBuffer virtual resize buffer method. #1128

Merged
merged 3 commits into from
Oct 12, 2021

Conversation

smichtch
Copy link
Contributor

@smichtch smichtch commented Oct 5, 2021

Small change to make Bond.IO.Safe.OutputBuffer memory allocation strategy extensible, for example using ArrayPool<byte>.Rent() instead of GC-heap memory.

Example of how this change can be leveraged:

internal sealed class ArrayPoolOutputBuffer : OutputBuffer, IDisposable
{
    private readonly ArrayPool<byte> _arrayPool;

    public ArrayPoolOutputBuffer(ArrayPool<byte> arrayPool, int length = 64 * 1024)
        : base(Initialize(arrayPool, length))
    {
        _arrayPool = arrayPool;
    }

    private static byte[] Initialize(ArrayPool<byte> arrayPool, int length)
    {
        arrayPool = arrayPool ?? throw new ArgumentNullException(nameof(arrayPool));

        if (length < 0)
        {
            throw new ArgumentOutOfRangeException(nameof(length));
        }

        return arrayPool.Rent(length);
    }

    protected override byte[] ResizeBuffer(byte[] buffer, int newSize)
    {
        var newBuffer = _arrayPool.Rent(newSize);

        if (buffer != null)
        {
            Buffer.BlockCopy(buffer, 0, newBuffer, 0, position);
            _arrayPool.Return(buffer);
        }

        return newBuffer;
    }

    public void Dispose()
    {
        if (buffer != null)
        {
            _arrayPool.Return(buffer);
            buffer = null;
        }
    }
}

@smichtch
Copy link
Contributor Author

smichtch commented Oct 5, 2021

I did not find tests that directly target OutputBuffer so I did not add any, presumably this path is hit by many other tests.

@smichtch smichtch force-pushed the output-buffer-virtual-grow branch from a6b84d8 to 28fd327 Compare October 5, 2021 20:51
@chwarr
Copy link
Member

chwarr commented Oct 5, 2021

Yup. A bunch of the tests use OutputBuffer incidentally.

Copy link
Member

@chwarr chwarr left a comment

Choose a reason for hiding this comment

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

Can you update the CHANGELOG.md file to include brief details about this? This is a minor change. (I don't think adding a virtual member breaks assembly binary compatibility.)

cs/src/core/io/safe/OutputBuffer.cs Outdated Show resolved Hide resolved
cs/src/core/io/safe/OutputBuffer.cs Show resolved Hide resolved
cs/src/core/io/safe/OutputBuffer.cs Show resolved Hide resolved
@smichtch smichtch force-pushed the output-buffer-virtual-grow branch from 28fd327 to 356d7c8 Compare October 6, 2021 17:34
@chwarr chwarr merged commit 3cf8f71 into microsoft:master Oct 12, 2021
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.

2 participants