Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use ByteString as container for websocket text message #1864

Merged
merged 6 commits into from
Jan 4, 2021

Conversation

robjtede
Copy link
Member

@robjtede robjtede commented Dec 31, 2020

PR Type

Improvement

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt

Overview

Use ByteString as the contained format for websocket text messages so that the underlying memory can be re-used when sending to many clients; eliminating need for more expensive String cloning.

Blocked on #1813 for bytestring v1.0 upgrade.

Breaking change to public interface.

Closes #1846.

@robjtede robjtede added B-semver-major breaking change requiring a major version bump A-http project: actix-http A-actors project: actix-web-actors labels Dec 31, 2020
@robjtede robjtede marked this pull request as draft December 31, 2020 03:27
@robjtede robjtede marked this pull request as ready for review January 4, 2021 00:57
@robjtede robjtede requested review from a team January 4, 2021 01:33
@fakeshadow
Copy link
Contributor

LGTM.
Btw well you are at it can we make Codec::new a const function? It would be useful if we make websocket extract type style where a const default config type would be ideal

@robjtede robjtede merged commit 7d632d0 into master Jan 4, 2021
@robjtede robjtede deleted the websocket-bytestring branch January 4, 2021 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-actors project: actix-web-actors A-http project: actix-http B-semver-major breaking change requiring a major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sending shared text messages to Websocket clients
3 participants