Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Adding Memory.Pin() to eventually replace Memory.Retain(bool) #17269

Merged
merged 3 commits into from
Mar 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions src/mscorlib/shared/System/Memory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,49 @@ public Span<T> Span
public bool TryCopyTo(Memory<T> destination) => Span.TryCopyTo(destination.Span);

/// <summary>
/// Returns a handle for the array.
/// <param name="pin">If pin is true, the GC will not move the array and hence its address can be taken</param>
/// Creates a handle for the memory.
/// The GC will not move the array until the returned <see cref="MemoryHandle"/>
Copy link
Member

Choose a reason for hiding this comment

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

The doc comment should not talk about arrays. In the OwnedMemory more there might not be an array.

Copy link
Author

@ahsonkhan ahsonkhan Mar 27, 2018

Choose a reason for hiding this comment

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

Agreed. The docs should also mention the possibility of GCHandle.Alloc throwing (An instance with nonprimitive (non-blittable) members cannot be pinned.).

I will address this separately after all the changes are in.

ReadOnlyMemory<object> memory = new object[10];
memory.Pin(); // throws ArgumentException

/// is disposed, enabling taking and using the memory's address.
/// </summary>
public unsafe MemoryHandle Pin()
Copy link
Member

Choose a reason for hiding this comment

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

Do we prefer the whole method to be unsafe, or just the part that does pointer arithmetic?

Copy link
Author

Choose a reason for hiding this comment

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

I kept the changes from Retain minimal. Also, we would end up with two unsafe blocks if we want to make this change (within each condition), which doesn't buy us much. Otherwise, it would be around the entire if block, which is essentially equivalent to it being around the whole method.

{
if (_index < 0)
{
return ((OwnedMemory<T>)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
}
else if (typeof(T) == typeof(char) && _object is string s)
{
// This case can only happen if a ReadOnlyMemory<char> was created around a string
// and then that was cast to a Memory<char> using unsafe / marshaling code. This needs
// to work, however, so that code that uses a single Memory<char> field to store either
// a readable ReadOnlyMemory<char> or a writable Memory<char> can still be pinned and
// used for interop purposes.
GCHandle handle = GCHandle.Alloc(s, GCHandleType.Pinned);
#if FEATURE_PORTABLE_SPAN
void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
#else
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
#endif // FEATURE_PORTABLE_SPAN
return new MemoryHandle(null, pointer, handle);
}
else if (_object is T[] array)
{
var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
#if FEATURE_PORTABLE_SPAN
void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
#else
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
#endif // FEATURE_PORTABLE_SPAN
return new MemoryHandle(null, pointer, handle);
}
return default;
}

/// <summary>[Obsolete, use Pin()] Creates a handle for the memory.</summary>
/// <param name="pin">
/// If pin is true, the GC will not move the array until the returned <see cref="MemoryHandle"/>
/// is disposed, enabling taking and using the memory's address.
/// </param>
public unsafe MemoryHandle Retain(bool pin = false)
{
MemoryHandle memoryHandle = default;
Expand Down
38 changes: 36 additions & 2 deletions src/mscorlib/shared/System/ReadOnlyMemory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,44 @@ public ReadOnlySpan<T> Span
/// <param name="destination">The span to copy items into.</param>
public bool TryCopyTo(Memory<T> destination) => Span.TryCopyTo(destination.Span);

/// <summary>Creates a handle for the memory.</summary>
/// <summary>
/// Creates a handle for the memory.
/// The GC will not move the array until the returned <see cref="MemoryHandle"/>
/// is disposed, enabling taking and using the memory's address.
/// </summary>
public unsafe MemoryHandle Pin()
{
if (_index < 0)
{
return ((OwnedMemory<T>)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf<T>());
}
else if (typeof(T) == typeof(char) && _object is string s)
{
GCHandle handle = GCHandle.Alloc(s, GCHandleType.Pinned);
#if FEATURE_PORTABLE_SPAN
void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
#else
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref s.GetRawStringData()), _index);
#endif // FEATURE_PORTABLE_SPAN
return new MemoryHandle(null, pointer, handle);
}
else if (_object is T[] array)
{
var handle = GCHandle.Alloc(array, GCHandleType.Pinned);
#if FEATURE_PORTABLE_SPAN
void* pointer = Unsafe.Add<T>((void*)handle.AddrOfPinnedObject(), _index);
#else
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index);
#endif // FEATURE_PORTABLE_SPAN
return new MemoryHandle(null, pointer, handle);
}
return default;
}

/// <summary>[Obsolete, use Pin()] Creates a handle for the memory.</summary>
/// <param name="pin">
/// If pin is true, the GC will not move the array until the returned <see cref="MemoryHandle"/>
/// is disposed, enabling the memory's address can be taken and used.
/// is disposed, enabling taking and using the memory's address.
/// </param>
public unsafe MemoryHandle Retain(bool pin = false)
{
Expand Down