Switch to Span/Memory internally

[roji]

Since it seems like we'll release Npgsql 4.0 at the same time that .NET Core 2.1 comes out, it may be a good idea to use the opportunity the internally switch to Span/Memory - especially in the read/write buffers. At the very least, this will give us access to very optimized primitive reading/writing routines (float, double, int, endian conversion...), and might help in some other areas as well.

@YohDeadfall what do you think?

[roji]

Note that for this to be worth it:

• The same code should work on previous versions of .NET Core and on .NET Framework (no big ifdefing)
• It shouldn't cause a perf degradation in platforms other than .NET Core 2.1 (at least nothing significant)

[roji]

Some thoughts on this...

• One big blocking issue is the lack of BCL support for Span in anything except .NET Core 2.1. For example, I don't think it's possible to do UTF8 decoding from a Span<byte> in .NET Standard 2.0, which probably makes this whole thing a non-starter (unless we go into complicated ifdefs). But maybe I missed something on this.
• Seems like the first place to do Span is in the TypeHandlers. For example, type handler would receive a Span<byte> for write and ReadOnlySpan<byte> for read. If nothing else, this would make reading safer as the provided ReadOnlySpan would be limited based on the column length, and we have less risk of going beyond the end.
• This works really nicely for simple type handlers (length is known in advance, data is always in memory) but it's more complicated for the complex type handlers which handle their own I/O. Instead of checking ReadByteLeft on the read buffer and doing Ensure(x), type handlers would likely call ReadMore(x) which would trigger I/O and return a new ReadOnlySpan (over the same byte[] of course).

[roji]

I just figured out that the lack of BCL support probably won't block us. Although it isn't possible to do UTF8 decoding/encoding directly on spans in .NET Standard 2.0, it's possible to get a byte* out of the span and call the pointer-based overloads for UTF8 decoding/encoding. So this should be fine.

I'll be working on implementing this soon.

[YohDeadfall]

I'm also planning to work on buffers and handlers to eliminate getting parameter lengths, but will start to do it in July. I think to design buffers like BufferWriter and BufferReader in the CoreFX Lab repo. @roji take a look on these structs and other things in the repo.

[roji]

@YohDeadfall thanks for the pointer, I'll take a closer look at the corefx lab repo.

For now I've concentrated on the simple type handlers, which simply receive a ReadOnlySpan<byte> for reading and Span<byte> for writing - this works quite well and improves the API considerably in my opinion.

The complex type API is of course a different matter, since type handlers need to be able to perform I/O themselves. I'll work on a good way of doing that and see if anything in corefx lab can be used.

Can you say a little bit about what you're planning to do with spans after the above?

[YohDeadfall]

Making high performance API is a good idea, but we shouldn't forget about usability. Therefore, my thoughts on this are:

1. Use an array pool (already added to .NET) to rent memory for buffers.
2. Use chained memory blocks when a whole large row should be in memory.
3. Redesign the reader and writer to be ref structs.
4. Add auto flush/ensure ability to them. There should be a private aggressively inlined method to advance the current position which checks the position and calls non-inlined Flush/Enshure.
5. The overridden Read method of the simple type handler base class should advance first and once, and pass a read only span to a new abstract method.
6. Same for writing, because simple types has constant lengths.
7. For variadic length fields the lenght should be calculated as a distance between the start and end positions.

Let's discuss the new API design first and do great things after. Probably @benaadams could help you too (:

[roji]

Good discussion. But before responding, there's one important thing I have to say.

I think that changing the type handler API to use Span is a great idea, but I don't expect it to significantly improve performance. Npgsql doesn't currently do any sort of slicing operation anywhere - it's already pretty much a zero-copy driver internally, in that values are simply read from a byte[] and returned to the user. So the usual perf gains (avoiding copying) don't apply here. The primary gains I see are:

1. The API is simply cleaner.
2. The API is safer. We currently give a type handler a buffer and a length, and trust it not to go out of bounds. Passing a span slice of the buffer restricts the handler to read only the actual binary data that makes up the value.
3. We can use some pretty optimized methods for reading/writing values (e.g. BinaryPrimitives, cast the Span<byte> into Span<double> and read, etc.

Anyway, here are some answers to your comments.

Use an array pool (already added to .NET) to rent memory for buffers.
Use chained memory blocks when a whole large row should be in memory.

If we keep Npgsql's architecture more or less as it is, I don't really see how buffer pooling would improve perf. The current strategy is simply to permanently attach buffers to physical database connections, which seems to work quite well - we don't need to synchronize in order to get a buffer, we simply use the connection's. One place where things might not be optimal is when the user is reading rows that are bigger than the buffer size, but the latter can be tweaked via the connection string (plus the user has the option to do sequential).

So unless we go down the relatively drastic route of implementing multiplexing (#1982), I don't see what problem array/buffer pooling would solve exactly. On the other hand if we do implement multiplexing, buffer pooling becomes a total necessity, since we can no longer buffer on the row - we need to buffer entire resultsets, since resultsets aren't read in order anymore.

Redesign the reader and writer to be ref structs.

Here I assume you're referring to reader/write over a span of bytes, which would allow reading/writing various types (int, float...) a bit like Npgsql's current read/write buffer? AFAIK these types don't exist yet, but I'm guessing you're referring to something in corefx labs.

Add auto flush/ensure ability to them. There should be a private aggressively inlined method to advance the current position which checks the position and calls non-inlined Flush/Ensure.

The API actually used to look like this a long time ago. The reason we don't do auto-flush/ensure, is that these are potentially (although rarely) operations that perform I/O. Because of the virality of async, this would make every single ReadInt() method async, since it would potentially need to read more data. Although the perf of non-yielding async methods has improved greatly, it still seems better to completely avoid async as much as possible by lifting the I/O operation as far up the stack as possible...

The overridden Read method of the simple type handler base class should advance first and once, and pass a read only span to a new abstract method.

This is pretty much what I've implemented in my branch (note that it's totally broken work-in-progress, but you can already see this specific change).

For variadic length fields the lenght should be calculated as a distance between the start and end positions.

You'll have to explain this one a bit more, I don't understand it.

[Scooletz]

Hey, is there any decision on working on accepting Span/Memory (more likely the latter, because of the async path)? I tried to follow the comments above but I failed at finding a definitive answer.

[roji]

@Scooletz, what exactly are you asking? This issue is about using Span internally inside Npgsql, when reading/writing data from the network. It doesn't affect user APIs in any way (that's another question).

Yes, the idea is definitely to switch to using Span, but this is part of a bigger change that may also include switching to the new pipelines API (see #2035). I don't foresee needing Memory (rather than Span) as it's all about populating a buffer in memory before performing the actual I/O write of sending it (or conversely, reading from buffers after they've been populated by an I/O read), so the slice of the buffer in question never needs to live in the heap.

[Scooletz]

Oh, I see @roji I totally misinterpreted the discussion and the issue then :-) Sorry for the harassment.

[NinoFloris]

Most of this is already done and the remaining bits are done in #5123