Runtime: API proposal: Add compression support in WebSocket

[zlatanov]

See discussion here dotnet/runtime#20004.

At the moment the WebSocket doesn't support per-message deflate (see https://tools.ietf.org/html/rfc7692#section-7). Adding support for it in the BCL will mean that people (myself including) will no longer resort to implementing custom WebSockets in order to use it.

Proposed API

/// <summary>
/// Options to enable per-message deflate compression for <seealso cref="WebSocket" />.
/// </summary>
public sealed class WebSocketCompressionOptions
{
    /// <summary>
    /// This parameter indicates the base-2 logarithm of the LZ77 sliding window size of the client context.
    /// Must be a value between 8 and 15 or -1 indicating no preferences. The default is -1.
    /// </summary>
    public int ClientMaxWindowBits { get; set; } = -1;

    /// <summary>
    /// When true, the client informs the peer server of a hint that even if the server doesn't include the
    /// "client_no_context_takeover" extension parameter in the corresponding
    /// extension negotiation response to the offer, the client is not going to use context takeover. The default is false.
    /// </summary>
    public bool ClientNoContextTakeover { get; set; }

    /// <summary>
    /// This parameter indicates the base-2 logarithm of the LZ77 sliding window size of the server context.
    /// Must be a value between 8 and 15 or -1 indicating no preferences. The default is -1.
    /// </summary>
    public int ServerMaxWindowBits { get; set; } = -1;

    /// <summary>
    /// When true, the client prevents the peer server from using context takeover. If the peer server doesn't use context
    /// takeover, the client doesn't need to reserve memory to retain the LZ77 sliding window between messages. The default is false.
    /// </summary>
    public bool ServerNoContextTakeover { get; set; }
}

public sealed class ClientWebSocketOptions
{
    /// <summary>
    /// Instructs the <seealso cref="ClientWebSocket" /> to try and negotiate per-message compression.
    /// </summary>
    public WebSocketCompressionOptions Compression { get; set; }
}

public enum WebSocketOutputCompression
{
    /// <summary>
    /// Enables output compression if the underlying <seealso cref="WebSocket" /> supports it.
    /// </summary>
    Default,

    /// <summary>
    /// Suppresses output compression for the next message.
    /// </summary>
    SuppressOne,

    /// <summary>
    /// Suppresses output compression.
    /// </summary>
    Suppress
}

public abstract class WebSocket
{
    /// <summary>
    /// Instructs the socket to compress (or not to) the messages being sent when
    /// compression has been successfully negotiated.
    /// </summary>
    public WebSocketOutputCompression OutputCompression { get; set; } 

    public static WebSocket CreateFromStream(Stream stream, bool isServer, string subProtocol, TimeSpan keepAliveInterval, WebSocketCompressionOptions compression);
}

Rationale and Usage

The main drive behind the API is that we should not introduce any breaking changes. WebSockets already built will work as is. This is why I suggest we add new CreateFromStream method with WebSocketCompressionOptions parameter instead of adding it to the existing one.

There are a few options built in the WebSocket compression protocol that are considered advance use - controlling the size of the LZ77 sliding window, context takeover. We could easily hide them and choose reasonable defaults, but I think there are good use cases for them and as such we should expose them. See this blog post for good example of their usage: https://www.igvita.com/2013/11/27/configuring-and-optimizing-websocket-compression/.

The usage of the WebSocket would not change at all. By default a WebSocket created with compression options would compress all messages if the connection on the other end supports it. We introduce OutputCompression property to allow explicit opt out. The property has no effect if compression is not supported for the current connection.

Here is example of how we would disable compression for specific messages:

var socket = GetWebSocket();

// Disable compression for the next message only
socket.OutputCompression = WebSocketCompressionOptions.SuppressOne;
await socket.SendAsync(...);

Here is example for WebSocketClient:

var client = new WebSocketClient();

// Indicate that we want compression if server supports it
client.Options.Compression = new WebSocketCompressionOptions();

await client.ConnectAsync(...);

// Same as before. If compression is enabled it will be performed automatically.
client.SendAsync(...); 

// If we need to explicitly disable compression for a time or specific messages
client.OutputCompression = WebSocketOutputCompression.Suppress;

// Send one or more messages
// ...

// Restore default compression state
client.OutputCompression = WebSocketOutputCompression.Default; 

Additional work will be required in AspNetCore repository and more specifically WebSocketMiddleware to light up the compression feature.

[davidfowl]

Something rubs me the wrong way about being able to mutate the options on the object after it's started on a per message basis. What's the rationale for that?

[zlatanov]

@davidfowl No particular reason. The options would actually be used only once in the WebSocket (in the ctor), no reference to them would be kept. In that regard they could very well be a struct, although it would have to be nullable in WebSocketClientOptions.

I find it more readable to be able to write

new WebSocketCompressionOptions
{
    ServerNoContextTakeover = true
}

than to write

new WebSocketCompressionOptions( serverNoContextTakeover: true );

With that in mind, this is only an opinion. If you think immutable class or struct is better for the case, I will change it.

[davidsh]

@dotnet/ncl

[scalablecory]

Would this still be usable if it were less configurable? What do we get by exposing the LZ77 window size?

[zlatanov]

@scalablecory The websocket would be very usable without it, but servers with high number of concurrent connections or clients with low memory requirements will have problems with any defaults we come up with.

The websocket will have to keep 2 separate native memory buffers for inflate and deflate. I've collected benchmark traces for both with the different options to illustrate the point.

Here is how the allocations look for inflate:

| Method | Window Bits | Allocated native memory |
|--------- |-----------:|------------------------:|
| Inflate | 8 | 8424 B |
| Inflate | 9 | 9448 B |
| Inflate | 10 | 11497 B |
| Inflate | 11 | 15592 B |
| Inflate | 12 | 23786 B |
| Inflate | 13 | 40177 B |
| Inflate | 14 | 72937 B |
| Inflate | 15 | 138472 B |

Here is how the allocations look for deflate. Here however we have another option to consider that I've not exposed - the memory and compression level and of the deflater (i.e. fast vs best).

The benchmark bellow is running in the default memory level 8 (max is 9) used by DeflateStream for best compression.

| Method | WindowBits | Allocated native memory |
|-------- |----------- |------------------------:|
| Deflate | 8 | 139240 B |
| Deflate | 15 | 268264 B |

The benchmark bellow is running in the default memory level 2.

| Method | WindowBits | Allocated native memory |
|-------- |----------- |------------------------:|
| Deflate | 8 | 10216B |
| Deflate | 15 | 139240 B |

As you can see the memory footprint for a single WebSocket can range anywhere between 18KB and 400KB. I cannot see how we can come up with any defaults that would be considered universally good enough for any web server or client.

[karelz]

@danmosemsft any idea why it was added to the Project ".NET Core impacting internal partners"?

[davidfowl]

@karelz that's where it most recently came up.

[zlatanov]

@karelz I see that you've changed the milestone to Future. Does it mean you will not consider this proposal for the 5.0 release?

[davidfowl]

I think it's worth doing if we can flesh out a design and @zlatanov contributes. @zlatanov do you still want to get this in for 5.0?

[zlatanov]

@davidfowl Definitely.

[karelz]

Milestone reflects our intent and priority from our side. It can change if more info appears. Nothing prevents anyone to get Future issues done in specific release, incl. 5.0. We take any high quality contributions.

API design is tricky though in general and requires non-trivial investment from our side first. My understanding was that the original API proposal here seemed to be too complicated to the person driving our API reviews -- @scalablecory can provide more feedback / impressions he has from the API.

@davidfowl what did you mean by "_that's where it most recently came up._"? Dan sent me offline info and it seems that it came from ASP.NET - maybe we can connect those dots also publicly here?

[davidfowl]

Milestone reflects our intent and priority from our side. It can change if more info appears. Nothing prevents anyone to get Future issues done in specific release, incl. 5.0. We take any high quality contributions.

👍

API design is tricky though in general and requires non-trivial investment from our side first. My understanding was that the original API proposal here seemed to be too complicated to the person driving our API reviews -- @scalablecory can provide more feedback / impressions he has from the API.

Sure and it has to be costed. That's basically what @zlatanov is looking for, design feedback following our typical API review process. We should spend a little time discussing it to see if it is complicated and make a decision. I don't think we have enough information to make a call yet (on the API).

@davidfowl what did you mean by "that's where it most recently came up."? Dan sent me offline info and it seems that it came from ASP.NET - maybe we can connect those dots also publicly here?

It didn't, it came from another team internally at Microsoft. This issue has come up in the past filed by customers on the ASP.NET repository and we opened an issue here https://github.com/dotnet/corefx/issues/15430 a long time ago.

[davidfowl]

@zlatanov Are you still going to take this on?

cc @BrennanConroy

[zlatanov]

@davidfowl Yup.

[zlatanov]

@davidfowl @karelz Now that the main branch is 6.0, will this be considered?

No feedback has been given since the API was proposed, almost 1 year ago (even just to say why it wasn't considered for 5.0 or at all). I'm sorry to say but it feels very off-putting.

[karelz]

@zlatanov yes, we plan to get back to this in 6.0. Sorry if it felt off-putting, but sadly, we have only limited bandwidth and even though this was important, it was less important than other features/APIs and we just didn't have anyone to do the research and provide valuable input.
This API was literally right below the cut line of 5.0 from the business perspective.

Given our current priorities (finishing off 5.0, YARP, QUIC), I think we should be able to get to this API in couple of months -- with plenty of time to finish it off in 6.0.
Let me know if you want to know more, or if I didn't fully answer your question / concerns. Thanks!

[jahmai]

@zlatanov I've been watching this for some time - look forward to you having the chance to get this out there 👍

[scalablecory]

Okay, all the API details proposed make sense now that I've skimmed the RFC.

I am not a web socket expert, so here are some thoughts -- @zlatanov @davidfowl does this make sense?:

• Is it valuable to keep the exact term NoContextTakeover? i.e. is it understood language that users would expect to carry across implementations? We may want to change this -- negative becomes positive ClientContextTakeover, or something a little more descriptive like ClientPersistDeflateContext.
• The WebSocketOutputCompression enum is a little weird -- it looks like we should put this in as a parameter to SendAsync instead.
• The memory savings are probably not important client-side, but I can see them being worthwhile server-side. I don't think it'll cost anything to enable for both, so we might as well, but if we want to be conservative we could probably leave the window/takeover settings off of ClientWebSocketOptions.

And general API review thoughts:

• To avoid adding even more constructors in future, we should consider creating a WebSocketCreationOptions class that is passed to WebSocket.CreateFromStream that we can add properties to in the future.
• Use nullables instead of -1 for window sizes.
• Fold the compression options into their parent types instead of introducing a new class. This is consistent with the approach we've taken with grouped settings in SocketsHttpHandler.

So the final API would look something like:

```c#
// new
public sealed class WebSocketCreationOptions
{
// taken from existing WebSocket.CreateFromStream params.
public bool IsServer { get; set; }
public string? SubProtocol { get; set; }
public TimeSpan KeepAliveInterval { get; set; }

// new
public bool UseDeflateCompression { get; set; } = false;
public int? ClientMaxDeflateWindowBits { get; set; } = null;
public bool ClientDeflateContextTakeover { get; set; } = true; // or ClientPersistDeflateContext etc.
public int? ServerMaxDeflateWindowBits { get; set; } = null;
public bool ServerDeflateContextTakeover { get; set; } = true;

}

// existing
public sealed class ClientWebSocketOptions
{
// new
public bool UseDeflateCompression { get; set; } = false;
public int? ClientMaxDeflateWindowBits { get; set; } = null;
public bool ClientDeflateContextTakeover { get; set; } = true; // or ClientPersistDeflateContext etc.
public int? ServerMaxDeflateWindowBits { get; set; } = null;
public bool ServerDeflateContextTakeover { get; set; } = true;
}

// existing
public abstract class WebSocket
{
// existing
public static WebSocket CreateFromStream(Stream stream, bool isServer, string? subProtocol, TimeSpan keepAliveInterval);

// new
public static WebSocket CreateFromStream(Stream stream, WebSocketCreationOptions options);

}
```

This appears straightforward to implement.

We can probably use the existing deflate code from System.IO.Compression. I think we'll want to add calls to inflateReset and deflateReset and maintain a pool of inflaters/deflaters for when the NoContextTakeover options are used, so we can realize the memory savings without going bonkers allocating new zlib contexts for every message. @carlossanlop @ericstj might have some guidance here.

[zlatanov]

@scalablecory thanks for the feedback. Here are my thoughts:

I completely agree that renaming some of the properties towards making more sense for consumers who are not (and probably should not) familiar with the RFC.

The implementation will be the same regardless server / client, but I agree that having this omitted from the client is a good thing, and basically matches what we have in the browsers right now where these options are handled by the browser itself.

Personally I am not fond of changing the API for sending messages. My reasoning is that consumers down the line don't need to be rebuild to support compression, one only need to change the setup of the websocket server / client. One other important fact about having a WebSocketMessageFlags.Compress flag in the API is that consumers must be aware when the socket really has compression enabled. For example a client might be created with compress options configured, but the server might not support compression or even it might decline it during the handshake. The opposite is true as well - the server might have compression enabled, but connected clients might not support it.

I suggested WebSocketOutputCompression enum to be able to temporarily suppress compression for messages where it doesn't make sense, but I can agree that omitting such feature is not a big issue. Again it will align with the browsers' implementations where once you have compression, all messages will be compressed.

[scalablecory]

Personally I am not fond of changing the API for sending messages. My reasoning is that consumers down the line don't need to be rebuild to support compression, one only need to change the setup of the websocket server / client. One other important fact about having a WebSocketMessageFlags.Compress flag in the API is that consumers must be aware when the socket really has compression enabled. For example a client might be created with compress options configured, but the server might not support compression or even it might decline it during the handshake. The opposite is true as well - the server might have compression enabled, but connected clients might not support it.

I suggested WebSocketOutputCompression enum to be able to temporarily suppress compression for messages where it doesn't make sense, but I can agree that omitting such feature is not a big issue. Again it will align with the browsers' implementations where once you have compression, all messages will be compressed.

Do you have some specific examples of where you'd want to disable compression? I agree it'd be easy to just leave it off here, but if there's a compelling use case we should keep it.

If we do want to keep it, we can invert the feature -- instead of WebSocketMessageFlags.Compress, use WebSocketMessageFlags.NoCompression. This way existing API users would just get the default of compress every message.

[zlatanov]

Do you have some specific examples of where you'd want to disable compression? I agree it'd be easy to just leave it off here, but if there's a compelling use case we should keep it.

If we do want to keep it, we can invert the feature -- instead of WebSocketMessageFlags.Compress, use WebSocketMessageFlags.NoCompression. This way existing API users would just get the default of compress every message.

Of top of my head when I experimented with this (and it was quite long ago), very small messages or messages containing unique data such as timestamps or guids benefited very little or even had negative gain with compression enabled. If this remains area of interest, I can setup a few benchmarks to find out whether and when it's useful.

In the real world though, in all times I've had compression enabled, it was all or none, I didn't bother trying to measure which messages would be better off without compression because eventually message structures evolve and this kind of measurement is very brittle.

I do like your suggestion of inverting the feature and using WebSocketMessageFlags.NoCompression though.

[scalablecory]

In the real world though, in all times I've had compression enabled, it was all or none, I didn't bother trying to measure which messages would be better off without compression because eventually message structures evolve and this kind of measurement is very brittle.

Okay. Easy thing to do here is nothing :). We can add that API later if someone finds a need.

[zlatanov]

@scalablecory I have one question about your suggested changes to the API proposal.

Isn't it better to have all compression options in the same object, rather than having them besides the existing ones. My reasoning is that it's not immediately obvious which options are related to the compression only, and having them configured without setting UseDeflateCompression is doing nothing.

```c#
public sealed class WebSocketCreationOptions
{
public bool IsServer { get; set; }
public string? SubProtocol { get; set; }
public TimeSpan KeepAliveInterval { get; set; }

// new
public bool UseDeflateCompression { get; set; } = false;
public int? ClientMaxDeflateWindowBits { get; set; } = null;
public bool ClientDeflateContextTakeover { get; set; } = true; 
public int? ServerMaxDeflateWindowBits { get; set; } = null;
public bool ServerDeflateContextTakeover { get; set; } = true;

}

``` c#
public sealed class WebSocketCreationOptions
{
    public bool IsServer { get; set; }
    public string? SubProtocol { get; set; }
    public TimeSpan KeepAliveInterval { get; set; }

    // new
    public WebSocketDefalateCompression? DeflateCompression { get; set; } 
}

In the second case we don't need UseDeflateCompression flag, because just having the object means we want compression. And the other benefit is that it's obvious what options towards compression are available. What are your thoughts?

[scalablecory]

We have some precedent for not grouping settings into their own objects, but I wouldn't mind it. I'll bring up both options during API review.

[davidfowl]

What happens if the server sends compressed messages and the client isn't aware? Is it gracefully handled?

cc @BrennanConroy @halter73 @Tratcher

[zlatanov]

@davidfowl What do you mean by that the client isn't aware? When a message is compressed, a bit flag is set in the message's headers. Also servers aren't supposed to send compressed messages if the handshake initiated by the client doesn't advocate that it wants it.

Other than that, if the server sends a compressed message, it's completely possible to read it as a normal message, and you will fail when trying to decode it as UTF-8 in case of a text message.

[scalablecory]

What happens if the server sends compressed messages and the client isn't aware? Is it gracefully handled?

As @zlatanov said -- it is negotiated and uncompressed messages are part of spec, so client will handle it gracefully.