Skip to content

Commit

Permalink
Cow-box WebSocketFrame.
Browse files Browse the repository at this point in the history
Motivation:

WebSocketFrameDecoder makes an allocation due to WebSocketFrame not fitting in a
existential. We should reduce this overhead.

Modifications:

Moved data and extensionData into a cow-box and reorder fields to more efficiently
pack them.

Result:

WebSocketFrame is now 14 bytes, down from 55.
  • Loading branch information
Marli Oshlack committed Nov 1, 2019
1 parent 62fd872 commit 9b58083
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 19 deletions.
78 changes: 61 additions & 17 deletions Sources/NIOWebSocket/WebSocketFrame.swift
Expand Up @@ -106,6 +106,19 @@ extension WebSocketMaskingKey: Collection {

/// A structured representation of a single WebSocket frame.
public struct WebSocketFrame {
private var _storage: WebSocketFrame._Storage

/// The masking key, if any.
///
/// A masking key is used to prevent specific byte sequences from appearing in the network
/// stream. This is primarily used by entities like browsers, but should be used any time it
/// is possible for a malicious user to control the data that appears in a websocket stream.
///
/// If this value is `nil`, and this frame was *received from* the network, the data in `data`
/// is not masked. If this value is `nil`, and this frame is being *sent to* the network, the
/// data in `data` will not be masked.
public var maskKey: WebSocketMaskingKey? = nil

/// Rather than unpack all the fields from the first byte, and thus take up loads
/// of storage in the structure, we keep them in their packed form in this byte and
/// use computed properties to unpack them.
Expand Down Expand Up @@ -170,30 +183,39 @@ public struct WebSocketFrame {
return data.readableBytes + (extensionData?.readableBytes ?? 0)
}

/// The masking key, if any.
///
/// A masking key is used to prevent specific byte sequences from appearing in the network
/// stream. This is primarily used by entities like browsers, but should be used any time it
/// is possible for a malicious user to control the data that appears in a websocket stream.
///
/// If this value is `nil`, and this frame was *received from* the network, the data in `data`
/// is not masked. If this value is `nil`, and this frame is being *sent to* the network, the
/// data in `data` will not be masked.
public var maskKey: WebSocketMaskingKey? = nil

/// The application data.
///
/// On frames received from the network, this data is not necessarily unmasked. This is to provide as much
/// information as possible. If unmasked data is desired, either use the computed `unmaskedData` property to
/// obtain it, or transform this data directly by calling `data.unmask(maskKey)`.
public var data: ByteBuffer
public var data: ByteBuffer {
get {
return self._storage.data
}
set {
if !isKnownUniquelyReferenced(&self._storage) {
self._storage = _Storage(copying: self._storage)
}
self._storage.data = newValue
}
}

/// The extension data, if any.
///
/// On frames received from the network, this data is not necessarily unmasked. This is to provide as much
/// information as possible. If unmasked data is desired, either use the computed `unmaskedExtensionData` property to
/// obtain it, or transform this data directly by calling `extensionData.unmask(maskKey)`.
public var extensionData: ByteBuffer? = nil
public var extensionData: ByteBuffer? {
get {
return self._storage.extensionData
}
set {
if !isKnownUniquelyReferenced(&self._storage) {
self._storage = _Storage(copying: self._storage)
}
self._storage.extensionData = newValue
}
}

/// The unmasked application data.
///
Expand Down Expand Up @@ -234,7 +256,7 @@ public struct WebSocketFrame {
/// - parameters:
/// - allocator: The `ByteBufferAllocator` to use when editing the empty buffers.
public init(allocator: ByteBufferAllocator) {
self.data = allocator.buffer(capacity: 0)
self._storage = .init(data: allocator.buffer(capacity: 0), extensionData: nil)
}

/// Create a `WebSocketFrame` with the given properties.
Expand All @@ -251,8 +273,7 @@ public struct WebSocketFrame {
public init(fin: Bool = false, rsv1: Bool = false, rsv2: Bool = false, rsv3: Bool = false,
opcode: WebSocketOpcode = .continuation, maskKey: WebSocketMaskingKey? = nil,
data: ByteBuffer, extensionData: ByteBuffer? = nil) {
self.data = data
self.extensionData = extensionData
self._storage = .init(data: data, extensionData: extensionData)
self.fin = fin
self.rsv1 = rsv1
self.rsv2 = rsv2
Expand All @@ -263,10 +284,33 @@ public struct WebSocketFrame {

/// Create a `WebSocketFrame` from the underlying data representation.
internal init(firstByte: UInt8, maskKey: WebSocketMaskingKey?, applicationData: ByteBuffer) {
self._storage = .init(data: applicationData, extensionData: nil)
self.firstByte = firstByte
self.maskKey = maskKey
self.data = applicationData
}
}

extension WebSocketFrame: Equatable {}

extension WebSocketFrame {
fileprivate class _Storage {
var data: ByteBuffer
var extensionData: ByteBuffer?

fileprivate init(data: ByteBuffer, extensionData: ByteBuffer?) {
self.data = data
self.extensionData = extensionData
}

fileprivate init(copying original: WebSocketFrame._Storage) {
self.data = original.data
self.extensionData = original.extensionData
}
}
}

extension WebSocketFrame._Storage: Equatable {
static func ==(lhs: WebSocketFrame._Storage, rhs: WebSocketFrame._Storage) -> Bool {
return lhs.data == rhs.data && lhs.extensionData == rhs.extensionData
}
}
2 changes: 1 addition & 1 deletion docker/docker-compose.1604.51.yaml
Expand Up @@ -30,7 +30,7 @@ services:
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_holding_buffer_with_space=2010
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_new_buffer=4010
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_new_buffer_with_space=4010
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2000
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=1000
- SANITIZER_ARG=--sanitize=thread

performance-test:
Expand Down
2 changes: 1 addition & 1 deletion docker/docker-compose.1804.50.yaml
Expand Up @@ -30,7 +30,7 @@ services:
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_holding_buffer_with_space=2010
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_new_buffer=4010
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_new_buffer_with_space=4010
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2000
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=1000

performance-test:
image: swift-nio:18.04-5.0
Expand Down

0 comments on commit 9b58083

Please sign in to comment.