From 9ea0ce61981aa09851c3d1e0a2b7dbd7f7a392f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 31 Oct 2022 22:52:28 +0100 Subject: [PATCH] Revert "perf(ext/websocket): optimize `op_ws_next_event` (#16325)" This reverts commit e3a30954811238ce55a9d0f8b193c3a58535b72d. --- ext/websocket/01_websocket.js | 40 ++++++----------- ext/websocket/02_websocketstream.js | 40 ++++++----------- ext/websocket/lib.rs | 66 ++++++++--------------------- 3 files changed, 43 insertions(+), 103 deletions(-) diff --git a/ext/websocket/01_websocket.js b/ext/websocket/01_websocket.js index 5b364f8095..52af4e7e5d 100644 --- a/ext/websocket/01_websocket.js +++ b/ext/websocket/01_websocket.js @@ -20,7 +20,6 @@ ArrayPrototypeJoin, ArrayPrototypeMap, ArrayPrototypeSome, - Uint32Array, ErrorPrototypeToString, ObjectDefineProperties, ObjectPrototypeIsPrototypeOf, @@ -85,10 +84,6 @@ const _idleTimeoutDuration = Symbol("[[idleTimeout]]"); const _idleTimeoutTimeout = Symbol("[[idleTimeoutTimeout]]"); const _serverHandleIdleTimeout = Symbol("[[serverHandleIdleTimeout]]"); - - /* [event type, close code] */ - const eventBuf = new Uint32Array(2); - class WebSocket extends EventTarget { [_rid]; @@ -417,15 +412,13 @@ async [_eventLoop]() { while (this[_readyState] !== CLOSED) { - const value = await core.opAsync( + const { kind, value } = await core.opAsync( "op_ws_next_event", this[_rid], - eventBuf, ); - const kind = eventBuf[0]; + switch (kind) { - /* string */ - case 0: { + case "string": { this[_serverHandleIdleTimeout](); const event = new MessageEvent("message", { data: value, @@ -434,8 +427,7 @@ this.dispatchEvent(event); break; } - /* binary */ - case 1: { + case "binary": { this[_serverHandleIdleTimeout](); let data; @@ -452,23 +444,18 @@ this.dispatchEvent(event); break; } - /* ping */ - case 3: { + case "ping": { core.opAsync("op_ws_send", this[_rid], { kind: "pong", }); break; } - /* pong */ - case 4: { + case "pong": { this[_serverHandleIdleTimeout](); break; } - /* closed */ - case 6: // falls through - /* close */ - case 2: { - const code = eventBuf[1]; + case "closed": + case "close": { const prevState = this[_readyState]; this[_readyState] = CLOSED; clearTimeout(this[_idleTimeoutTimeout]); @@ -478,8 +465,8 @@ await core.opAsync( "op_ws_close", this[_rid], - code, - value, + value.code, + value.reason, ); } catch { // ignore failures @@ -488,15 +475,14 @@ const event = new CloseEvent("close", { wasClean: true, - code, - reason: value, + code: value.code, + reason: value.reason, }); this.dispatchEvent(event); core.tryClose(this[_rid]); break; } - /* error */ - case 5: { + case "error": { this[_readyState] = CLOSED; const errorEv = new ErrorEvent("error", { diff --git a/ext/websocket/02_websocketstream.js b/ext/websocket/02_websocketstream.js index 876f4d0ed7..598816d052 100644 --- a/ext/websocket/02_websocketstream.js +++ b/ext/websocket/02_websocketstream.js @@ -27,7 +27,6 @@ SymbolFor, TypeError, Uint8ArrayPrototype, - Uint32Array, } = window.__bootstrap.primordials; webidl.converters.WebSocketStreamOptions = webidl.createDictionaryConverter( @@ -169,15 +168,12 @@ PromisePrototypeThen( (async () => { while (true) { - const kind = new Uint32Array(2); - await core.opAsync( + const { kind } = await core.opAsync( "op_ws_next_event", create.rid, - kind, ); - /* close */ - if (kind[0] === 2) { + if (kind === "close") { break; } } @@ -241,46 +237,35 @@ await this.closed; }, }); - const pull = async (controller) => { - /* [event type, close code] */ - const eventBuf = new Uint32Array(2); - const value = await core.opAsync( + const { kind, value } = await core.opAsync( "op_ws_next_event", this[_rid], - eventBuf, ); - const kind = eventBuf[0]; + switch (kind) { - /* string */ - case 0: { + case "string": { controller.enqueue(value); break; } - /* binary */ - case 1: { + case "binary": { controller.enqueue(value); break; } - /* ping */ - case 3: { + case "ping": { await core.opAsync("op_ws_send", this[_rid], { kind: "pong", }); await pull(controller); break; } - /* closed */ - case 6: // falls through - /* close */ - case 2: { - const code = eventBuf[1]; - this[_closed].resolve({ code, reason: value }); + case "closed": + case "close": { + this[_closed].resolve(value); core.tryClose(this[_rid]); break; } - /* error */ - case 5: { + case "error": { const err = new Error(value); this[_closed].reject(err); controller.error(err); @@ -300,8 +285,7 @@ return pull(controller); } - const code = eventBuf[1]; - this[_closed].resolve({ code, reason: value }); + this[_closed].resolve(value); core.tryClose(this[_rid]); } }; diff --git a/ext/websocket/lib.rs b/ext/websocket/lib.rs index 61bc444592..984d39e9dc 100644 --- a/ext/websocket/lib.rs +++ b/ext/websocket/lib.rs @@ -20,7 +20,6 @@ use deno_core::OpState; use deno_core::RcRef; use deno_core::Resource; use deno_core::ResourceId; -use deno_core::StringOrBuffer; use deno_core::ZeroCopyBuf; use deno_tls::create_client_config; use http::header::HeaderName; @@ -562,23 +561,11 @@ pub enum NextEventResponse { Closed, } -#[repr(u32)] -enum NextEventKind { - String = 0, - Binary = 1, - Close = 2, - Ping = 3, - Pong = 4, - Error = 5, - Closed = 6, -} - -#[op(deferred)] +#[op] pub async fn op_ws_next_event( state: Rc>, rid: ResourceId, - kind_out: &mut [u32], -) -> Result, AnyError> { +) -> Result { let resource = state .borrow_mut() .resource_table @@ -586,45 +573,28 @@ pub async fn op_ws_next_event( let cancel = RcRef::map(&resource, |r| &r.cancel); let val = resource.next_message(cancel).await?; - let (kind, value) = match val { - Some(Ok(Message::Text(text))) => ( - NextEventKind::String as u32, - Some(StringOrBuffer::String(text)), - ), - Some(Ok(Message::Binary(data))) => ( - NextEventKind::Binary as u32, - Some(StringOrBuffer::Buffer(data.into())), - ), - Some(Ok(Message::Close(Some(frame)))) => { - let code: u16 = frame.code.into(); - kind_out[1] = code as u32; - ( - NextEventKind::Close as u32, - Some(StringOrBuffer::String(frame.reason.to_string())), - ) - } - Some(Ok(Message::Close(None))) => { - kind_out[1] = 1005; - ( - NextEventKind::Close as u32, - Some(StringOrBuffer::String(String::new())), - ) - } - Some(Ok(Message::Ping(_))) => (NextEventKind::Ping as u32, None), - Some(Ok(Message::Pong(_))) => (NextEventKind::Pong as u32, None), - Some(Err(e)) => ( - NextEventKind::Error as u32, - Some(StringOrBuffer::String(e.to_string())), - ), + let res = match val { + Some(Ok(Message::Text(text))) => NextEventResponse::String(text), + Some(Ok(Message::Binary(data))) => NextEventResponse::Binary(data.into()), + Some(Ok(Message::Close(Some(frame)))) => NextEventResponse::Close { + code: frame.code.into(), + reason: frame.reason.to_string(), + }, + Some(Ok(Message::Close(None))) => NextEventResponse::Close { + code: 1005, + reason: String::new(), + }, + Some(Ok(Message::Ping(_))) => NextEventResponse::Ping, + Some(Ok(Message::Pong(_))) => NextEventResponse::Pong, + Some(Err(e)) => NextEventResponse::Error(e.to_string()), None => { // No message was received, presumably the socket closed while we waited. // Try close the stream, ignoring any errors, and report closed status to JavaScript. let _ = state.borrow_mut().resource_table.close(rid); - (NextEventKind::Closed as u32, None) + NextEventResponse::Closed } }; - kind_out[0] = kind as u32; - Ok(value) + Ok(res) } pub fn init(