-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Documentation for Memory<T>, IMemoryOwner<T> #548
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
Conversation
<remarks> | ||
<format type="text/markdown"><![CDATA[ | ||
|
||
The `IMemoryOwner<T>` interface is used to define the owner responsible for the lifetime management of a <xref:System.Memory%601> buffer. An instance of the `IMemoryOwner<T>` interface is returned by the <xref:System.Buffers.ArrayPool%601.Rent%2A?displayProperty=nameWithType> and <xef:System.Buffers.MemoryPool%601.Rent%2A?displayProperty=nameWithType> methods. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't return IMemoryOwner from ArrayPool.Rent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But MemoryPool.Rent does, just to clarify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also fix from xef to xref
<summary>To be added.</summary> | ||
<value>To be added.</value> | ||
<summary>Gets the memory belonging to this owner.</summary> | ||
<value>The memory belonging to this owner.</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remarks field still states "To be added."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ahsonkhan If any field has "to be added." then it will not be displayed in the published article. It's a "magic" value.
xml/System/Memory`1.xml
Outdated
<remarks> | ||
<format type="text/markdown"><![CDATA[ | ||
|
||
If `array` is `null`, this constructor returns a `null` <xref:System.Memory%601> object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Memory<T>
is a struct and can't be null. So stating that it returns a null object could cause confusion. It returns the default struct.
xml/System/Memory`1.xml
Outdated
]]></format> | ||
</remarks> | ||
<exception cref="T:System.ArrayTypeMismatchException"> | ||
<paramref name="T" /> is a reference type, and <paramref name="array" /> is not an array of type <paramref name="T" />.</exception> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be useful to mention that this could occur due to co-variant arrays?
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/covariance-contravariance/
<param name="other">The object to compare with the current instance.</param> | ||
<summary>Determines whether the specified <see cref="T:System.Memory`1" /> object is equal to the current object.</summary> | ||
<returns><see langword="true" /> if the current instance and <paramref name="other" /> are equal; otherwise, <see langword="false" />. </returns> | ||
<remarks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add a remark to clarify that this is purely reference equality and we are not comparing the elements of Memory<T>
for equality.
xml/System/Memory`1.xml
Outdated
<param name="start">The index at which to begin the slice.</param> | ||
<param name="length">The number of elements to include in the slice.</param> | ||
<summary>Forms a slice out of the current memory starting at a specified index for a specified length.</summary> | ||
<returns>An object that <paramref name="length" /> elements from the current instance starting at <paramref name="start" />. </returns> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentence is awkward to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary really explains it best. Return should just be shortened to something like "Memory object contain X elements from the current instance.
xml/System/Span`1.xml
Outdated
|
||
## Span\<T> and slices | ||
|
||
`Span\<T>` includes two overloads of the <xref:System.Span%601.Slice%2A> method that forms a slice out of the current span that starts at a specified index. This makes it possible to treat the data in a `Span\<T>` as a set of logical chunks that can be processed as needed by portions of a data processing pipeline with minimal performance impact. For example, since modern server protocols are often text-based, manipulation of strings and substrings is particularly important. In the <xref:System.String> class, the major method for extracting substrings is <xref:System.String.Substring%2A>. For data pipelines that rely on extensive string manipulation, it use offers some performance penalties, since it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For data pipelines that rely on extensive string manipulation, it use offers some performance penalties, since it
... its use offers...
<returns>To be added.</returns> | ||
<param name="array">The array to convert.</param> | ||
<summary>Defines an implicit conversion of an array to a <see cref="T:System.Memory`1" /> object.</summary> | ||
<returns>The converted object.</returns> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be added
remarks here and elsewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine @ahsonkhan. It's the default value and is not rendered on docs.
xml/System/Memory`1.xml
Outdated
|
||
## Remarks | ||
|
||
Like <xref:System.Span%601>, `Memory<T>` represents a contiguous region of memory. Unlike <xref:System.Span%601>, however, `Memory<T>` is not a [ref struct](~/doocs/csharp/reference-semantics-with-value-types?view=netcore-2.1.md#ref-struct-type). This means that `Memory<T>` can be backed by: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that...
That difference, struct vs ref struct, really just impacts whether or not it can be stored on the heap, which in turn impacts whether it can be used as a field in a class, whether it can used across await and yield boundaries, etc.
xml/System/Memory`1.xml
Outdated
- A string (a sequence of characters). | ||
- A <xref:System.Runtime.InteropServices.SafeHandle>. | ||
|
||
`Memory<T>` cannot be backed by transient unmanaged memory, such as memory available by using [stackalloc](~/docs/csharp/language-reference/keywords/stackalloc.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really true. With IMemoryPool<T>
, a Memory<T>
can be created that's backed pretty much any contiguous memory, regardless of where it came from: if you can get a pointer to it, you can create a Memory<T>
for it... it just requires some more work to do.
xml/System/Memory`1.xml
Outdated
|
||
`Memory<T>` cannot be backed by transient unmanaged memory, such as memory available by using [stackalloc](~/docs/csharp/language-reference/keywords/stackalloc.md). | ||
|
||
Because the memory allocated to the `Memory<T>` structure is placed on the managed heap, it does not have the same restrictions as a <xref:System.Span%601> instance. In particular: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't quite right. Memory<T>
is just a struct... it could live on the managed heap (as a field on a class), but it could also, for example, be on the stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this restriction (on span) related to the fact that it's a ref struct? So memory not being a ref struct, doesn't have those restrictions?
xml/System/Memory`1.xml
Outdated
<remarks> | ||
<format type="text/markdown"><![CDATA[ | ||
|
||
If `array` is `null`, this constructor returns a `null` <xref:System.Memory%601> object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per Ahson's comments earlier, you don't get back null
... you get back default(Memory<T>)
, i.e. basically a zero'd-out Memory<T>
.
xml/System/Memory`1.xml
Outdated
|
||
- An array. | ||
- A string (a sequence of characters). | ||
- A <xref:System.Runtime.InteropServices.SafeHandle>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SafeHandle?
|
||
]]></format> | ||
|
||
</remarks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we'll have a subsequent PR that adds the relevant safety rules related to IMemoryOwner<T>
, e.g. it shouldn't be disposed of while a reference to its memory is handed out, which generally means it shouldn't have a finalizer, etc.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have a place where these safety rules are discussed @stephentoub?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor fixes. One bad link though
xml/System/Memory`1.xml
Outdated
|
||
## Remarks | ||
|
||
Like <xref:System.Span%601>, `Memory<T>` represents a contiguous region of memory. Unlike <xref:System.Span%601>, however, `Memory<T>` is not a [ref struct](~/doocs/csharp/reference-semantics-with-value-types?view=netcore-2.1.md#ref-struct-type). This means that `Memory<T>` can be placed on the managed heap, whereas <xref:System.Span%601> cannot. As a result, the `Memory<T>` structure does not have the same restrictions as a <xref:System.Span%601> instance. In particular: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it says /doocs
instead of /docs
xml/System/Memory`1.xml
Outdated
<returns>To be added.</returns> | ||
<remarks>To be added.</remarks> | ||
<summary>Creates a handle for the <see cref="T:System.Memory`1" /> object.</summary> | ||
<returns>a handle for the <see cref="T:System.Memory`1" /> object.</returns> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capitol A at the start
Closing and reopening to begin new build. |
Documentation for Memory<T>, IMemoryOwner<T>
ReadOnlyMemory<T> will be documented after review comments are incorporated into Memory<T>.
@stephentoub @ahsonkhan
Related to dotnet/samples#444
Fixes dotnet/docs#4400