From 498f6ad431478f655b136782093e19e29248b24d Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 14 Aug 2019 19:54:35 -0400 Subject: [PATCH] Remove dead code: legacy read/write ops (#2776) readSync and writeSync use dispatch_minimal now. --- cli/main.rs | 1 - cli/msg.fbs | 22 -------- cli/{ => ops}/dispatch_minimal.rs | 74 ++++---------------------- cli/ops/files.rs | 86 ------------------------------- cli/ops/io.rs | 43 ++++++++++++++++ cli/ops/mod.rs | 28 ++++++---- js/dispatch.ts | 6 ++- js/dispatch_minimal.ts | 13 +++++ js/files.ts | 56 ++++++-------------- 9 files changed, 106 insertions(+), 223 deletions(-) rename cli/{ => ops}/dispatch_minimal.rs (58%) create mode 100644 cli/ops/io.rs diff --git a/cli/main.rs b/cli/main.rs index 52d08ea9e9c26f..020ace5a3e47ea 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -21,7 +21,6 @@ pub mod deno_dir; pub mod deno_error; pub mod diagnostics; mod disk_cache; -mod dispatch_minimal; mod file_fetcher; pub mod flags; pub mod fmt_errors; diff --git a/cli/msg.fbs b/cli/msg.fbs index 8fb92fee9f01c6..def17fed3f0df3 100644 --- a/cli/msg.fbs +++ b/cli/msg.fbs @@ -48,10 +48,8 @@ union Any { PermissionRevoke, Permissions, PermissionsRes, - Read, ReadDir, ReadDirRes, - ReadRes, Readlink, ReadlinkRes, Remove, @@ -83,8 +81,6 @@ union Any { WorkerGetMessage, WorkerGetMessageRes, WorkerPostMessage, - Write, - WriteRes, } enum ErrorKind: byte { @@ -491,24 +487,6 @@ table OpenRes { rid: uint32; } -table Read { - rid: uint32; - // (ptr, len) is passed as second parameter to Deno.core.send(). -} - -table ReadRes { - nread: uint; - eof: bool; -} - -table Write { - rid: uint32; -} - -table WriteRes { - nbyte: uint; -} - table Close { rid: uint32; } diff --git a/cli/dispatch_minimal.rs b/cli/ops/dispatch_minimal.rs similarity index 58% rename from cli/dispatch_minimal.rs rename to cli/ops/dispatch_minimal.rs index 322094be2d880a..be2cafea4a9f93 100644 --- a/cli/dispatch_minimal.rs +++ b/cli/ops/dispatch_minimal.rs @@ -7,14 +7,11 @@ use crate::state::ThreadSafeState; use deno::Buf; use deno::CoreOp; +use deno::ErrBox; use deno::Op; -use deno::OpId; use deno::PinnedBuf; use futures::Future; -const OP_READ: OpId = 1; -const OP_WRITE: OpId = 2; - #[derive(Copy, Clone, Debug, PartialEq)] // This corresponds to RecordMinimal on the TS side. pub struct Record { @@ -72,21 +69,23 @@ fn test_parse_min_record() { assert_eq!(parse_min_record(&buf), None); } -pub fn dispatch_minimal( +pub type MinimalOp = dyn Future + Send; +pub type Dispatcher = fn(i32, Option) -> Box; + +pub fn dispatch( + d: Dispatcher, state: &ThreadSafeState, - op_id: OpId, - mut record: Record, + control: &[u8], zero_copy: Option, ) -> CoreOp { + let mut record = parse_min_record(control).unwrap(); let is_sync = record.promise_id == 0; - let min_op = match op_id { - OP_READ => ops::read(record.arg, zero_copy), - OP_WRITE => ops::write(record.arg, zero_copy), - _ => unimplemented!(), - }; let state = state.clone(); + let rid = record.arg; + let min_op = d(rid, zero_copy); + let fut = Box::new(min_op.then(move |result| -> Result { match result { Ok(r) => { @@ -109,54 +108,3 @@ pub fn dispatch_minimal( Op::Async(fut) } } - -mod ops { - use crate::deno_error; - use crate::resources; - use crate::tokio_write; - use deno::ErrBox; - use deno::PinnedBuf; - use futures::Future; - - type MinimalOp = dyn Future + Send; - - pub fn read(rid: i32, zero_copy: Option) -> Box { - debug!("read rid={}", rid); - let zero_copy = match zero_copy { - None => { - return Box::new( - futures::future::err(deno_error::no_buffer_specified()), - ) - } - Some(buf) => buf, - }; - match resources::lookup(rid as u32) { - None => Box::new(futures::future::err(deno_error::bad_resource())), - Some(resource) => Box::new( - tokio::io::read(resource, zero_copy) - .map_err(ErrBox::from) - .and_then(move |(_resource, _buf, nread)| Ok(nread as i32)), - ), - } - } - - pub fn write(rid: i32, zero_copy: Option) -> Box { - debug!("write rid={}", rid); - let zero_copy = match zero_copy { - None => { - return Box::new( - futures::future::err(deno_error::no_buffer_specified()), - ) - } - Some(buf) => buf, - }; - match resources::lookup(rid as u32) { - None => Box::new(futures::future::err(deno_error::bad_resource())), - Some(resource) => Box::new( - tokio_write::write(resource, zero_copy) - .map_err(ErrBox::from) - .and_then(move |(_resource, _buf, nwritten)| Ok(nwritten as i32)), - ), - } - } -} diff --git a/cli/ops/files.rs b/cli/ops/files.rs index ce328562359ce6..e311db425ea63c 100644 --- a/cli/ops/files.rs +++ b/cli/ops/files.rs @@ -8,7 +8,6 @@ use crate::ops::serialize_response; use crate::ops::CliOpResult; use crate::resources; use crate::state::ThreadSafeState; -use crate::tokio_write; use deno::*; use flatbuffers::FlatBufferBuilder; use futures::Future; @@ -119,91 +118,6 @@ pub fn op_close( } } -pub fn op_read( - _state: &ThreadSafeState, - base: &msg::Base<'_>, - data: Option, -) -> CliOpResult { - let cmd_id = base.cmd_id(); - let inner = base.inner_as_read().unwrap(); - let rid = inner.rid(); - - match resources::lookup(rid) { - None => Err(deno_error::bad_resource()), - Some(resource) => { - let op = tokio::io::read(resource, data.unwrap()) - .map_err(ErrBox::from) - .and_then(move |(_resource, _buf, nread)| { - let builder = &mut FlatBufferBuilder::new(); - let inner = msg::ReadRes::create( - builder, - &msg::ReadResArgs { - nread: nread as u32, - eof: nread == 0, - }, - ); - Ok(serialize_response( - cmd_id, - builder, - msg::BaseArgs { - inner: Some(inner.as_union_value()), - inner_type: msg::Any::ReadRes, - ..Default::default() - }, - )) - }); - if base.sync() { - let buf = op.wait()?; - Ok(Op::Sync(buf)) - } else { - Ok(Op::Async(Box::new(op))) - } - } - } -} - -pub fn op_write( - _state: &ThreadSafeState, - base: &msg::Base<'_>, - data: Option, -) -> CliOpResult { - let cmd_id = base.cmd_id(); - let inner = base.inner_as_write().unwrap(); - let rid = inner.rid(); - - match resources::lookup(rid) { - None => Err(deno_error::bad_resource()), - Some(resource) => { - let op = tokio_write::write(resource, data.unwrap()) - .map_err(ErrBox::from) - .and_then(move |(_resource, _buf, nwritten)| { - let builder = &mut FlatBufferBuilder::new(); - let inner = msg::WriteRes::create( - builder, - &msg::WriteResArgs { - nbyte: nwritten as u32, - }, - ); - Ok(serialize_response( - cmd_id, - builder, - msg::BaseArgs { - inner: Some(inner.as_union_value()), - inner_type: msg::Any::WriteRes, - ..Default::default() - }, - )) - }); - if base.sync() { - let buf = op.wait()?; - Ok(Op::Sync(buf)) - } else { - Ok(Op::Async(Box::new(op))) - } - } - } -} - pub fn op_seek( _state: &ThreadSafeState, base: &msg::Base<'_>, diff --git a/cli/ops/io.rs b/cli/ops/io.rs new file mode 100644 index 00000000000000..6102389420fe8e --- /dev/null +++ b/cli/ops/io.rs @@ -0,0 +1,43 @@ +use super::dispatch_minimal::MinimalOp; +use crate::deno_error; +use crate::resources; +use crate::tokio_write; +use deno::ErrBox; +use deno::PinnedBuf; +use futures::Future; + +pub fn op_read(rid: i32, zero_copy: Option) -> Box { + debug!("read rid={}", rid); + let zero_copy = match zero_copy { + None => { + return Box::new(futures::future::err(deno_error::no_buffer_specified())) + } + Some(buf) => buf, + }; + match resources::lookup(rid as u32) { + None => Box::new(futures::future::err(deno_error::bad_resource())), + Some(resource) => Box::new( + tokio::io::read(resource, zero_copy) + .map_err(ErrBox::from) + .and_then(move |(_resource, _buf, nread)| Ok(nread as i32)), + ), + } +} + +pub fn op_write(rid: i32, zero_copy: Option) -> Box { + debug!("write rid={}", rid); + let zero_copy = match zero_copy { + None => { + return Box::new(futures::future::err(deno_error::no_buffer_specified())) + } + Some(buf) => buf, + }; + match resources::lookup(rid as u32) { + None => Box::new(futures::future::err(deno_error::bad_resource())), + Some(resource) => Box::new( + tokio_write::write(resource, zero_copy) + .map_err(ErrBox::from) + .and_then(move |(_resource, _buf, nwritten)| Ok(nwritten as i32)), + ), + } +} diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index 021c0fa471a1e4..bf621e0e136753 100644 --- a/cli/ops/mod.rs +++ b/cli/ops/mod.rs @@ -1,7 +1,5 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. use crate::deno_error::GetErrorKind; -use crate::dispatch_minimal::dispatch_minimal; -use crate::dispatch_minimal::parse_min_record; use crate::msg; use crate::state::ThreadSafeState; use crate::tokio_util; @@ -13,12 +11,15 @@ use hyper; use hyper::rt::Future; use tokio_threadpool; +mod dispatch_minimal; +mod io; + mod compiler; use compiler::{op_cache, op_fetch_source_file}; mod errors; use errors::{op_apply_source_map, op_format_error}; mod files; -use files::{op_close, op_open, op_read, op_seek, op_write}; +use files::{op_close, op_open, op_seek}; mod fetch; use fetch::op_fetch; mod fs; @@ -71,6 +72,8 @@ fn empty_buf() -> Buf { } const FLATBUFFER_OP_ID: OpId = 44; +const OP_READ: OpId = 1; +const OP_WRITE: OpId = 2; pub fn dispatch_all( state: &ThreadSafeState, @@ -81,11 +84,18 @@ pub fn dispatch_all( ) -> CoreOp { let bytes_sent_control = control.len(); let bytes_sent_zero_copy = zero_copy.as_ref().map(|b| b.len()).unwrap_or(0); - let op = if op_id != FLATBUFFER_OP_ID { - let min_record = parse_min_record(control).unwrap(); - dispatch_minimal(state, op_id, min_record, zero_copy) - } else { - dispatch_all_legacy(state, control, zero_copy, op_selector) + + let op = match op_id { + OP_READ => { + dispatch_minimal::dispatch(io::op_read, state, control, zero_copy) + } + OP_WRITE => { + dispatch_minimal::dispatch(io::op_write, state, control, zero_copy) + } + FLATBUFFER_OP_ID => { + dispatch_all_legacy(state, control, zero_copy, op_selector) + } + _ => panic!("bad op_id"), }; state.metrics_op_dispatched(bytes_sent_control, bytes_sent_zero_copy); op @@ -226,7 +236,6 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { msg::Any::Open => Some(op_open), msg::Any::PermissionRevoke => Some(op_revoke_permission), msg::Any::Permissions => Some(op_permissions), - msg::Any::Read => Some(op_read), msg::Any::ReadDir => Some(op_read_dir), msg::Any::Readlink => Some(op_read_link), msg::Any::Remove => Some(op_remove), @@ -245,7 +254,6 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { msg::Any::Truncate => Some(op_truncate), msg::Any::HomeDir => Some(op_home_dir), msg::Any::Utime => Some(op_utime), - msg::Any::Write => Some(op_write), // TODO(ry) split these out so that only the appropriate Workers can access // them. diff --git a/js/dispatch.ts b/js/dispatch.ts index babea5739d3b54..6edd2981cb7c94 100644 --- a/js/dispatch.ts +++ b/js/dispatch.ts @@ -32,9 +32,13 @@ function flatbufferRecordFromBuf(buf: Uint8Array): FlatbufferRecord { } export function handleAsyncMsgFromRust(opId: number, ui8: Uint8Array): void { - const buf32 = new Int32Array(ui8.buffer, ui8.byteOffset, ui8.byteLength / 4); if (opId !== FLATBUFFER_OP_ID) { // Fast and new + const buf32 = new Int32Array( + ui8.buffer, + ui8.byteOffset, + ui8.byteLength / 4 + ); const recordMin = recordFromBufMinimal(opId, buf32); handleAsyncMsgFromRustMinimal(ui8, recordMin); } else { diff --git a/js/dispatch_minimal.ts b/js/dispatch_minimal.ts index df0a290b22f806..483342127c0a50 100644 --- a/js/dispatch_minimal.ts +++ b/js/dispatch_minimal.ts @@ -52,6 +52,19 @@ export function handleAsyncMsgFromRustMinimal( promise!.resolve(result); } +export function sendSyncMinimal( + opId: number, + arg: number, + zeroCopy: Uint8Array +): number { + scratch32[0] = 0; // promiseId 0 indicates sync + scratch32[1] = arg; + const res = core.dispatch(opId, scratchBytes, zeroCopy)!; + const res32 = new Int32Array(res.buffer, res.byteOffset, 3); + const resRecord = recordFromBufMinimal(opId, res32); + return resRecord.result; +} + export function sendAsyncMinimal( opId: number, arg: number, diff --git a/js/files.ts b/js/files.ts index eb899d738d0a00..1582d8fd48d661 100644 --- a/js/files.ts +++ b/js/files.ts @@ -11,11 +11,12 @@ import { SyncSeeker } from "./io"; import * as dispatch from "./dispatch"; -import { sendAsyncMinimal } from "./dispatch_minimal"; +import { sendAsyncMinimal, sendSyncMinimal } from "./dispatch_minimal"; import * as msg from "gen/cli/msg_generated"; import { assert } from "./util"; import * as flatbuffers from "./flatbuffers"; +// Warning: These constants defined in two places. Here and in cli/ops/mod.rs. const OP_READ = 1; const OP_WRITE = 2; @@ -62,26 +63,6 @@ export async function open( return resOpen(await dispatch.sendAsync(...reqOpen(filename, mode))); } -function reqRead( - rid: number, - p: Uint8Array -): [flatbuffers.Builder, msg.Any, flatbuffers.Offset, Uint8Array] { - const builder = flatbuffers.createBuilder(); - const inner = msg.Read.createRead(builder, rid); - return [builder, msg.Any.Read, inner, p]; -} - -function resRead(baseRes: null | msg.Base): number | EOF { - assert(baseRes != null); - assert(msg.Any.ReadRes === baseRes!.innerType()); - const res = new msg.ReadRes(); - assert(baseRes!.inner(res) != null); - if (res.eof()) { - return EOF; - } - return res.nread(); -} - /** Read synchronously from a file ID into an array buffer. * * Return `number | EOF` for the operation. @@ -93,7 +74,14 @@ function resRead(baseRes: null | msg.Base): number | EOF { * */ export function readSync(rid: number, p: Uint8Array): number | EOF { - return resRead(dispatch.sendSync(...reqRead(rid, p))); + const nread = sendSyncMinimal(OP_READ, rid, p); + if (nread < 0) { + throw new Error("read error"); + } else if (nread == 0) { + return EOF; + } else { + return nread; + } } /** Read from a file ID into an array buffer. @@ -118,23 +106,6 @@ export async function read(rid: number, p: Uint8Array): Promise { } } -function reqWrite( - rid: number, - p: Uint8Array -): [flatbuffers.Builder, msg.Any, flatbuffers.Offset, Uint8Array] { - const builder = flatbuffers.createBuilder(); - const inner = msg.Write.createWrite(builder, rid); - return [builder, msg.Any.Write, inner, p]; -} - -function resWrite(baseRes: null | msg.Base): number { - assert(baseRes != null); - assert(msg.Any.WriteRes === baseRes!.innerType()); - const res = new msg.WriteRes(); - assert(baseRes!.inner(res) != null); - return res.nbyte(); -} - /** Write synchronously to the file ID the contents of the array buffer. * * Resolves with the number of bytes written. @@ -145,7 +116,12 @@ function resWrite(baseRes: null | msg.Base): number { * Deno.writeSync(file.rid, data); */ export function writeSync(rid: number, p: Uint8Array): number { - return resWrite(dispatch.sendSync(...reqWrite(rid, p))); + let result = sendSyncMinimal(OP_WRITE, rid, p); + if (result < 0) { + throw new Error("write error"); + } else { + return result; + } } /** Write to the file ID the contents of the array buffer.