Remove dead code: legacy read/write ops (#2776)

ry · web-flow · commit 498f6ad43147 · 2019-08-14T19:54:35.000-04:00
readSync and writeSync use dispatch_minimal now.
diff --git 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
@@ -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/ops/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<Item = i32, Error = ErrBox> + Send;
+pub type Dispatcher = fn(i32, Option<PinnedBuf>) -> Box<MinimalOp>;
+
+pub fn dispatch(
+  d: Dispatcher,
   state: &ThreadSafeState,
-  op_id: OpId,
-  mut record: Record,
+  control: &[u8],
   zero_copy: Option<PinnedBuf>,
 ) -> 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<Buf, ()> {
     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<Item = i32, Error = ErrBox> + Send;
-
-  pub fn read(rid: i32, zero_copy: Option<PinnedBuf>) -> Box<MinimalOp> {
-    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<PinnedBuf>) -> Box<MinimalOp> {
-    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
@@ -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<PinnedBuf>,
-) -> 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<PinnedBuf>,
-) -> 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
@@ -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<PinnedBuf>) -> Box<MinimalOp> {
+  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<PinnedBuf>) -> Box<MinimalOp> {
+  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
@@ -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<CliDispatchFn> {
     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<CliDispatchFn> {
     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
@@ -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
diff --git a/js/files.ts b/js/files.ts
