Skip to content

Commit 524dd4a

Browse files
committed
Avoid making a copy of large frames.
This isn't very significant compared to the cost of compression. It can make a real difference for decompression.
1 parent baadc33 commit 524dd4a

File tree

4 files changed

+49
-14
lines changed

4 files changed

+49
-14
lines changed

docs/project/changelog.rst

+14
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,20 @@ Backwards-incompatible changes
6161
Aliases for deprecated API were removed from ``__all__``. As a consequence,
6262
they cannot be imported e.g. with ``from websockets import *`` anymore.
6363

64+
.. admonition:: :attr:`Frame.data <frames.Frame.data>` is now a bytes-like object.
65+
:class: note
66+
67+
In addition to :class:`bytes`, it may be a :class:`bytearray` or a
68+
:class:`memoryview`.
69+
70+
If you wrote an :class:`extension <extensions.Extension>` that relies on
71+
methods not provided by these new types, you may need to update your code.
72+
73+
Improvements
74+
............
75+
76+
* Sending or receiving large compressed frames is now faster.
77+
6478
.. _13.1:
6579

6680
13.1

src/websockets/extensions/permessage_deflate.py

+20-10
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,22 @@ def decode(
129129
# Uncompress data. Protect against zip bombs by preventing zlib from
130130
# decompressing more than max_length bytes (except when the limit is
131131
# disabled with max_size = None).
132-
data = frame.data
133-
if frame.fin:
134-
data += _EMPTY_UNCOMPRESSED_BLOCK
132+
if frame.fin and len(frame.data) < 2044:
133+
# Profiling shows that appending four bytes, which makes a copy, is
134+
# faster than calling decompress() again when data is less than 2kB.
135+
data = bytes(frame.data) + _EMPTY_UNCOMPRESSED_BLOCK
136+
else:
137+
data = frame.data
135138
max_length = 0 if max_size is None else max_size
136139
try:
137140
data = self.decoder.decompress(data, max_length)
141+
if self.decoder.unconsumed_tail:
142+
raise PayloadTooBig(f"over size limit (? > {max_size} bytes)")
143+
if frame.fin and len(frame.data) >= 2044:
144+
# This cannot generate additional data.
145+
self.decoder.decompress(_EMPTY_UNCOMPRESSED_BLOCK)
138146
except zlib.error as exc:
139147
raise ProtocolError("decompression failed") from exc
140-
if self.decoder.unconsumed_tail:
141-
raise PayloadTooBig(f"over size limit (? > {max_size} bytes)")
142148

143149
# Allow garbage collection of the decoder if it won't be reused.
144150
if frame.fin and self.remote_no_context_takeover:
@@ -176,11 +182,15 @@ def encode(self, frame: frames.Frame) -> frames.Frame:
176182

177183
# Compress data.
178184
data = self.encoder.compress(frame.data) + self.encoder.flush(zlib.Z_SYNC_FLUSH)
179-
if frame.fin and data[-4:] == _EMPTY_UNCOMPRESSED_BLOCK:
180-
# Making a copy is faster than memoryview(a)[:-4] until about 2kB.
181-
# On larger messages, it's slower but profiling shows that it's
182-
# marginal compared to compress() and flush(). Keep it simple.
183-
data = data[:-4]
185+
if frame.fin:
186+
# Sync flush generates between 5 or 6 bytes, ending with the bytes
187+
# 0x00 0x00 0xff 0xff, which must be removed.
188+
assert data[-4:] == _EMPTY_UNCOMPRESSED_BLOCK
189+
# Making a copy is faster than memoryview(a)[:-4] until 2kB.
190+
if len(data) < 2048:
191+
data = data[:-4]
192+
else:
193+
data = memoryview(data)[:-4]
184194

185195
# Allow garbage collection of the encoder if it won't be reused.
186196
if frame.fin and self.local_no_context_takeover:

src/websockets/frames.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import secrets
88
import struct
99
from collections.abc import Generator, Sequence
10-
from typing import Callable
10+
from typing import Callable, Union
1111

1212
from .exceptions import PayloadTooBig, ProtocolError
1313

@@ -139,7 +139,7 @@ class Frame:
139139
"""
140140

141141
opcode: Opcode
142-
data: bytes
142+
data: Union[bytes, bytearray, memoryview]
143143
fin: bool = True
144144
rsv1: bool = False
145145
rsv2: bool = False
@@ -160,7 +160,7 @@ def __str__(self) -> str:
160160
if self.opcode is OP_TEXT:
161161
# Decoding only the beginning and the end is needlessly hard.
162162
# Decode the entire payload then elide later if necessary.
163-
data = repr(self.data.decode())
163+
data = repr(bytes(self.data).decode())
164164
elif self.opcode is OP_BINARY:
165165
# We'll show at most the first 16 bytes and the last 8 bytes.
166166
# Encode just what we need, plus two dummy bytes to elide later.
@@ -178,7 +178,7 @@ def __str__(self) -> str:
178178
# binary. If self.data is a memoryview, it has no decode() method,
179179
# which raises AttributeError.
180180
try:
181-
data = repr(self.data.decode())
181+
data = repr(bytes(self.data).decode())
182182
coding = "text"
183183
except (UnicodeDecodeError, AttributeError):
184184
binary = self.data

tests/extensions/test_permessage_deflate.py

+11
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import dataclasses
2+
import os
23
import unittest
34

45
from websockets.exceptions import (
@@ -167,6 +168,16 @@ def test_encode_decode_fragmented_binary_frame(self):
167168
self.assertEqual(dec_frame1, frame1)
168169
self.assertEqual(dec_frame2, frame2)
169170

171+
def test_encode_decode_large_frame(self):
172+
# There is a separate code path that avoids copying data
173+
# when frames are larger than 2kB. Test it for coverage.
174+
frame = Frame(OP_BINARY, os.urandom(4096))
175+
176+
enc_frame = self.extension.encode(frame)
177+
dec_frame = self.extension.decode(enc_frame)
178+
179+
self.assertEqual(dec_frame, frame)
180+
170181
def test_no_decode_text_frame(self):
171182
frame = Frame(OP_TEXT, "café".encode())
172183

0 commit comments

Comments
 (0)