-
Notifications
You must be signed in to change notification settings - Fork 344
Comments on Span<T> Spec #1653
Comments
Why should it? Using
That example does look weird. And it's IMO pointless, the first example is enough. And no, the stack allocation doesn't survive.
It's both a matter of safety and a necessity (API's like
The magic is that there is a
An alias for an object reference? That's a rather limited description of
All multi field structs are tearable and they have always been, there's no way around that.
The C# spec should help with this and the following question: https://github.com/dotnet/csharplang/blob/master/proposals/span-safety.md
It doesn't. The document is pretty clear in this regard, there are cases where somehting that doesn't have the stack restriction of Span is needed. |
Makes sense. I suggest adding this note to the document.
Substantially less characters to type (unsafe{},ToPointer()) when using Marshal? Both Marshal.AllocHGlobal and IntPtr have always been part of the framework so I doubt people (who use these) would mistakenly consider these “safe”.
That example needs to be removed.
So if I understand correctly: A “ref field” sets a requirement on the field type to be stack only? Can you please explain why this was needed for the inner working of the Span? What processing flow benefits from it?
Your right. I largely oversimplified the description of the ref keyword. Nevertheless, I don’t get the idea of "interior pointer" I mean who benefits from the second level of indirection and how? Specifically, this paragraph is unclear not me:
Regarding:
This is a good document and it helps understand what a ref-like is. I might have some comments on the content but I would need to read it a couple of times and think some more about the various scenarios outlined. Nevertheless, the spec should spell this out correctly: “ref-like” – a particular set of requirements on a type. I highly recommend merging the Span spec document with this document or at least have the spec hyperlink to it as it contains important context the reader needs to be aware of to fully understand the Span construct.
Personally I would prefer having one construct over two where there is no difference in the overall intent. Perhaps it would better to have only a single Class named ‘Span’ providing advanced developers the option to give it stack semantics by marking it as such, something like:
or
In any case ‘Memory’ sounds too general to me and the name doesn’t specifically hint a relationship with Span. Perhaps HeapSpan would be a better name.
The link is broken. |
Doesn't sound like a good enough reason to me. The proper solution at the framework level would be to provide some sort of native memory management APIs. AFAIK there already are some experiments in this area ( https://github.com/dotnet/corefxlab/blob/master/src/System.Buffers.Experimental/System/Buffers/OwnedNativeBuffer.cs for example).
"field type" is ambiguous. If you meant "field's declaring type" then yes, any type containing a ref field is stack only.
Pay attention to the fast/slow representations. The slow representation has to keep an object reference ( The fast representation is trivial, it only contains a managed pointer (
This is basically work in progress. The whole ref-like thing didn't even exist when the span effort started.
It's complicated. That one is a language spec and this one is the spec of a framework (and framework to some extent) feature. And both are work in progress. And it's not always clear what goes where (language/framework/runtime spec).
They're different.
Work in progress. Memory is now Buffer so it's here: https://github.com/dotnet/corefxlab/blob/master/src/System.Buffers.Primitives/System/Buffers/Buffer.cs. There are various issues opened about its design and even about updating documents. |
Perhaps we can reach middle ground with a Constructor overload.
I'm going to take a step back here. I naïvely assumed that these documents are considered kinda “final” since all the past "work in progress" designs I looked at where in “issues” (not files). I recommend stating “Work in progress” at the very beginning of these spec documents. I would be happy to review their updated versions when they mature. I would highly appreciate a ping to this issue when the documents are revised or even better if you guys can open a new read-only repo just for announcements. Currently, I don’t think there is an option to subscribe for change notifications on a file level. Thanks @mikedn! |
Closing this discussion issue as Span is now in the corefx repo for .NET Core 2.1. Future issues should be opened in the dotnet/corefx repo for System.Span. https://blogs.msdn.microsoft.com/dotnet/2017/11/15/welcome-to-c-7-2-and-span/ |
Regarding:
https://github.com/dotnet/corefxlab/blob/master/docs/specs/span.md
This wording suggest to me that Span somehow can represent several distinct regions as one.
Consider rephrasing:
"which represents a contiguous region of arbitrary memory"
(T already implies different memory block sizes)
Furthermore, since the context is memory, as reader, my initial concern was about the generality of T
Is T constrained to primitive types, Structs with only primitive types? Consider adding a following explanatory note
The snippet reads a bit odd, why is Span required to be created in an Unsafe Context?
In other words: why doesn't the Span take an IntPtr? (Is there a way to allocate unmanaged heap memory without Marshal.AllocHGlobal?)
Is there any disadvantage in passing an IntPtr and then calling ToPointer() ?
According to
https://github.com/dotnet/coreclr/blob/cdb827b6cf72bdb8b4d0dbdaec160c32de7c185f/src/mscorlib/src/System/IntPtr.cs
The size of IntPtr equals pointer
Furthermore, I would avoid, in examples, variable names which suggest acronyms (e.g "nativeSpan") and use a more instructive naming, something like:
"spanOverHeapMemory"
BTW, The comment and Marshal.AllocHGlobal already suggest unmanaged memory
Same goes for the stack example:
stackSpan->spanOverStackMemory
The scenario is well known (No need ASP.NET reassurance). The snippet is bad, it's too involved and assumes ASP.NET specific abstractions.
It's not clear how 128 bytes stack memory plays a role here and how all desired bytes are ultimately combined to one uint in case 1 byte arrives then another byte arrives, and so on...
Furthermore, it’s not explained how stack allocated memory survives when the method completes.
In simple words: What’s going on here?
Consider replacing with a simple parser example (like UIntParser) without ASP.NET kung-fu
I don't think it's matter of safety, but rather of a necessity.. I mean unless all Api's are Span aware how can you avoid the copy? Please detail one or two representing scenarios out of the many .
I don’t understand. Please elaborate what problem/optimization this new magic “ref field” solves in the context of GC collection and promotion. I assume the average .net developer (myself included) understands ref as a way to create an alias for an object reference and it's implemented via pointer-to-pointer. What the new semantics? Please describe the benefit as a sequence of GC events along a timeline.
Is there a new type called "tearable struct type" or are you referring to the "atomicity challenge" concerning multi-field structs as members in general?
What exactly is this "by-ref" type? How is it different from a "reference type" or passing a reference type or primitive type be ref
Please elaborate this seems like new lifetime semantics.
Memory<T>
This reads like "Span 2.0". If so, why do we need
Span<T>
?If
Memory<T>
supersededSpan<T>
please note that at the beginning of the document so the reader can skip it all together.The text was updated successfully, but these errors were encountered: