Skip to content

Update from_buffer_copy to accept an IBufferProtocol#1405

Merged
slozier merged 3 commits intoIronLanguages:masterfrom
slozier:from_buffer_copy
Apr 19, 2022
Merged

Update from_buffer_copy to accept an IBufferProtocol#1405
slozier merged 3 commits intoIronLanguages:masterfrom
slozier:from_buffer_copy

Conversation

@slozier
Copy link
Copy Markdown
Contributor

@slozier slozier commented Apr 17, 2022

Part of #1404

Copy link
Copy Markdown
Member

@BCSharp BCSharp left a comment

Choose a reason for hiding this comment

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

The changes are fine except for WriteSpan used now in several places. The use is OK but the implementation of method itself caught my attention. It is much less efficient than what spans allow. Perhaps it can be rewritten to something like:

internal unsafe void WriteSpan(int offset, ReadOnlySpan<byte> value) {
    Span<byte> dest = new Span<byte>((void*)_data, _size).Slice(offset);
    value.CopyTo(dest);
    GC.KeepAlive(this);
}

@BCSharp BCSharp self-requested a review April 19, 2022 05:39
Copy link
Copy Markdown
Member

@BCSharp BCSharp left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@slozier slozier merged commit 9503e1d into IronLanguages:master Apr 19, 2022
@slozier slozier deleted the from_buffer_copy branch April 19, 2022 12:44
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