Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(567)

Issue 2260073002: RFC: Sending pre-encoded text over a web socket. (Closed)

Created:
4 years, 4 months ago by rmacnak
Modified:
4 years, 4 months ago
CC:
reviews_dartlang.org, turnidge, vm-dev_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Add WebSocket.addUtf8Text to allow sending pre-encoded text without a round-trip UTF-8 conversion. Use it to implement the vm-service, where in particular we are concerned about the space overhead of the conversion leading to the process being killed on iOS. Closes #27129 R=johnmccutchan@google.com, lrn@google.com Committed: https://github.com/dart-lang/sdk/commit/aa38062a239c7507fa9a902f53473b2cc6b150cb

Patch Set 1 #

Patch Set 2 : . #

Total comments: 4

Patch Set 3 : as addUtf8Text #

Total comments: 2

Patch Set 4 : error message #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -10 lines) Patch
M CHANGELOG.md View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M runtime/bin/vmservice/server.dart View 1 2 1 chunk +1 line, -4 lines 0 comments Download
M sdk/lib/io/websocket.dart View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
M sdk/lib/io/websocket_impl.dart View 1 2 3 3 chunks +18 lines, -6 lines 0 comments Download

Messages

Total messages: 20 (5 generated)
rmacnak
4 years, 4 months ago (2016-08-19 00:28:14 UTC) #2
Cutch
Alternatively, we could just provide an explicit API: enum WebSocketMessageType { TEXT, BINARY } void ...
4 years, 4 months ago (2016-08-19 13:43:31 UTC) #3
zra
On 2016/08/19 13:43:31, Cutch wrote: > Alternatively, we could just provide an explicit API: > ...
4 years, 4 months ago (2016-08-19 15:41:38 UTC) #4
rmacnak
Updated to the sendMessage variant. There's no public path between the _WebSocketImpl and the _WebSocketOutgoingTransform, ...
4 years, 4 months ago (2016-08-19 20:37:53 UTC) #5
siva
Ryan can you post numbers indicating the actual memory footprint reduction you see using the ...
4 years, 4 months ago (2016-08-19 20:52:54 UTC) #7
rmacnak
On 2016/08/19 20:52:54, siva wrote: > Ryan can you post numbers indicating the actual memory ...
4 years, 4 months ago (2016-08-19 21:46:28 UTC) #8
Cutch
lgtm
4 years, 4 months ago (2016-08-23 23:13:29 UTC) #10
Lasse Reichstein Nielsen
https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart File sdk/lib/io/websocket.dart (right): https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart#newcode419 sdk/lib/io/websocket.dart:419: void sendMessage(WebSocketMessageType type, List<int> bytes); For consistency, the name ...
4 years, 4 months ago (2016-08-24 05:10:12 UTC) #11
Lasse Reichstein Nielsen
https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart File sdk/lib/io/websocket.dart (right): https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart#newcode419 sdk/lib/io/websocket.dart:419: void sendMessage(WebSocketMessageType type, List<int> bytes); Or maybe addUtf8Text - ...
4 years, 4 months ago (2016-08-24 05:51:04 UTC) #12
Cutch
https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart File sdk/lib/io/websocket.dart (right): https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart#newcode419 sdk/lib/io/websocket.dart:419: void sendMessage(WebSocketMessageType type, List<int> bytes); On 2016/08/24 05:51:04, Lasse ...
4 years, 4 months ago (2016-08-24 14:01:41 UTC) #13
Lasse Reichstein Nielsen
https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart File sdk/lib/io/websocket.dart (right): https://codereview.chromium.org/2260073002/diff/20001/sdk/lib/io/websocket.dart#newcode419 sdk/lib/io/websocket.dart:419: void sendMessage(WebSocketMessageType type, List<int> bytes); That's a perfectly reasonable ...
4 years, 4 months ago (2016-08-24 14:16:48 UTC) #14
rmacnak
Recast as addUtf8Text. PTAL.
4 years, 4 months ago (2016-08-24 17:49:27 UTC) #16
Lasse Reichstein Nielsen
lgtm https://chromiumcodereview.appspot.com/2260073002/diff/40001/sdk/lib/io/websocket_impl.dart File sdk/lib/io/websocket_impl.dart (right): https://chromiumcodereview.appspot.com/2260073002/diff/40001/sdk/lib/io/websocket_impl.dart#newcode1187 sdk/lib/io/websocket_impl.dart:1187: throw new ArgumentError(bytes); throw new ArgumentError.value(bytes, "bytes", "Is ...
4 years, 4 months ago (2016-08-24 20:12:46 UTC) #17
rmacnak
https://chromiumcodereview.appspot.com/2260073002/diff/40001/sdk/lib/io/websocket_impl.dart File sdk/lib/io/websocket_impl.dart (right): https://chromiumcodereview.appspot.com/2260073002/diff/40001/sdk/lib/io/websocket_impl.dart#newcode1187 sdk/lib/io/websocket_impl.dart:1187: throw new ArgumentError(bytes); On 2016/08/24 20:12:46, Lasse Reichstein Nielsen ...
4 years, 4 months ago (2016-08-24 23:14:43 UTC) #18
rmacnak
4 years, 4 months ago (2016-08-24 23:15:14 UTC) #20
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as
aa38062a239c7507fa9a902f53473b2cc6b150cb (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698