refactor(core): simplify Deno.core initialisation, remove stale TODO (#8847)

This commit rewrites initialisation of the "shared queue" and
in effect prevents from double execution of "core/core.js" and
"core/error.js".

Previously both of these files were executed every time a "JsRuntime"
was created. That lead to a situation where one copy of each script
was included in the snapshot and then another copy would be
executed after loading the snapshot.

Effectively "JsRuntime::shared_init" was removed; instead execution
of those scripts and actual initialisation of shared queue
was split into two helper functions: "JsRuntime::js_init" and
"JsRuntime::share_queue_init".

Additionally stale TODO comments were removed.

cli/tsc/99_main_compiler.js

@@ -597,7 +597,6 @@ delete Object.prototype.__proto__;
     hasStarted = true;
     languageService = ts.createLanguageService(host);
     core.ops();
-    core.registerErrorClass("Error", Error);
     setLogDebug(debugFlag, "TSLS");
     debug("serverInit()");
   }
@@ -613,7 +612,6 @@ delete Object.prototype.__proto__;
     }
     hasStarted = true;
     core.ops();
-    core.registerErrorClass("Error", Error);
     setLogDebug(!!debugFlag, "TS");
   }
 

core/core.js

@@ -34,17 +34,9 @@ SharedQueue Binary Layout
 
   let asyncHandlers;
 
-  let initialized = false;
   let opsCache = {};
   const errorMap = {};
 
-  function maybeInit() {
-    if (!initialized) {
-      init();
-      initialized = true;
-    }
-  }
-
   function init() {
     const shared = core.shared;
     assert(shared.byteLength > 0);
@@ -72,14 +64,12 @@ SharedQueue Binary Layout
   }
 
   function reset() {
-    maybeInit();
     shared32[INDEX_NUM_RECORDS] = 0;
     shared32[INDEX_NUM_SHIFTED_OFF] = 0;
     shared32[INDEX_HEAD] = HEAD_INIT;
   }
 
   function head() {
-    maybeInit();
     return shared32[INDEX_HEAD];
   }
 
@@ -160,7 +150,6 @@ SharedQueue Binary Layout
   }
 
   function setAsyncHandler(opId, cb) {
-    maybeInit();
     assert(opId != null);
     asyncHandlers[opId] = cb;
   }
@@ -273,6 +262,7 @@ SharedQueue Binary Layout
     resources,
     registerErrorClass,
     getErrorClass,
+    sharedQueueInit: init,
     // sharedQueue is private but exposed for testing.
     sharedQueue: {
       MAX_RECORDS,

core/error.rs

@@ -200,8 +200,6 @@ impl JsError {
           .ok();
       let stack = stack.map(|s| s.to_rust_string_lossy(scope));
 
-      // FIXME(bartlmieju): the rest of this function is CLI only
-
       // Read an array of structured frames from error.__callSiteEvals.
       let frames_v8 = get_property(scope, exception, "__callSiteEvals");
       let frames_v8: Option<v8::Local<v8::Array>> =
@@ -383,7 +381,6 @@ pub(crate) fn attach_handle_to_error(
   err: AnyError,
   handle: v8::Local<v8::Value>,
 ) -> AnyError {
-  // TODO(bartomieju): this is a special case...
   ErrWithV8Handle::new(scope, err, handle).into()
 }
 

core/ops.rs

@@ -41,11 +41,8 @@ pub struct OpState {
   gotham_state: GothamState,
 }
 
-impl Default for OpState {
-  // TODO(ry) Only deno_core should be able to construct an OpState. But I don't
-  // know how to make default private. Maybe rename to
-  //   pub(crate) fn new() -> OpState
-  fn default() -> OpState {
+impl OpState {
+  pub(crate) fn new() -> OpState {
     OpState {
       resource_table: Default::default(),
       op_table: OpTable::default(),
@@ -119,7 +116,7 @@ impl Default for OpTable {
 
 #[test]
 fn op_table() {
-  let state = Rc::new(RefCell::new(OpState::default()));
+  let state = Rc::new(RefCell::new(OpState::new()));
 
   let foo_id;
   let bar_id;

core/runtime.rs

@@ -81,7 +81,6 @@ pub struct JsRuntime {
   v8_isolate: Option<v8::OwnedIsolate>,
   snapshot_creator: Option<v8::SnapshotCreator>,
   has_snapshotted: bool,
-  needs_init: bool,
   allocations: IsolateAllocations,
 }
 
@@ -203,6 +202,8 @@ impl JsRuntime {
       unsafe { v8_init() };
     });
 
+    let has_startup_snapshot = options.startup_snapshot.is_some();
+
     let global_context;
     let (mut isolate, maybe_snapshot_creator) = if options.will_snapshot {
       // TODO(ry) Support loading snapshots before snapshotting.
@@ -258,7 +259,7 @@ impl JsRuntime {
     let js_error_create_fn = options
       .js_error_create_fn
       .unwrap_or_else(|| Rc::new(JsError::create));
-    let mut op_state = OpState::default();
+    let mut op_state = OpState::new();
 
     if let Some(get_error_class_fn) = options.get_error_class_fn {
       op_state.get_error_class_fn = get_error_class_fn;
@@ -286,13 +287,22 @@ impl JsRuntime {
       waker: AtomicWaker::new(),
     })));
 
-    Self {
+    let mut js_runtime = Self {
       v8_isolate: Some(isolate),
       snapshot_creator: maybe_snapshot_creator,
       has_snapshotted: false,
-      needs_init: true,
       allocations: IsolateAllocations::default(),
+    };
+
+    if !has_startup_snapshot {
+      js_runtime.js_init();
     }
+
+    if !options.will_snapshot {
+      js_runtime.shared_queue_init();
+    }
+
+    js_runtime
   }
 
   pub fn global_context(&mut self) -> v8::Global<v8::Context> {
@@ -322,17 +332,29 @@ impl JsRuntime {
     s.clone()
   }
 
-  /// Executes a bit of built-in JavaScript to provide Deno.sharedQueue.
-  fn shared_init(&mut self) {
-    if self.needs_init {
-      self.needs_init = false;
-      self
-        .execute("deno:core/core.js", include_str!("core.js"))
-        .unwrap();
-      self
-        .execute("deno:core/error.js", include_str!("error.js"))
-        .unwrap();
-    }
+  /// Executes a JavaScript code to provide Deno.core and error reporting.
+  ///
+  /// This function can be called during snapshotting.
+  fn js_init(&mut self) {
+    self
+      .execute("deno:core/core.js", include_str!("core.js"))
+      .unwrap();
+    self
+      .execute("deno:core/error.js", include_str!("error.js"))
+      .unwrap();
+  }
+
+  /// Executes a JavaScript code to initialize shared queue binding
+  /// between Rust and JS.
+  ///
+  /// This function mustn't be called during snapshotting.
+  fn shared_queue_init(&mut self) {
+    self
+      .execute(
+        "deno:core/shared_queue_init.js",
+        "Deno.core.sharedQueueInit()",
+      )
+      .unwrap();
   }
 
   /// Returns the runtime's op state, which can be used to maintain ops
@@ -356,8 +378,6 @@ impl JsRuntime {
     js_filename: &str,
     js_source: &str,
   ) -> Result<(), AnyError> {
-    self.shared_init();
-
     let context = self.global_context();
 
     let scope = &mut v8::HandleScope::with_context(self.v8_isolate(), context);
@@ -482,8 +502,6 @@ impl JsRuntime {
     &mut self,
     cx: &mut Context,
   ) -> Poll<Result<(), AnyError>> {
-    self.shared_init();
-
     let state_rc = Self::state(self.v8_isolate());
     {
       let state = state_rc.borrow();
@@ -740,8 +758,6 @@ impl JsRuntime {
     load_id: ModuleLoadId,
     id: ModuleId,
   ) -> Result<(), AnyError> {
-    self.shared_init();
-
     let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
     let context1 = self.global_context();
@@ -827,8 +843,6 @@ impl JsRuntime {
     &mut self,
     id: ModuleId,
   ) -> mpsc::Receiver<Result<(), AnyError>> {
-    self.shared_init();
-
     let state_rc = Self::state(self.v8_isolate());
     let context = self.global_context();
 
@@ -1276,7 +1290,6 @@ impl JsRuntime {
     specifier: &ModuleSpecifier,
     code: Option<String>,
   ) -> Result<ModuleId, AnyError> {
-    self.shared_init();
     let loader = {
       let state_rc = Self::state(self.v8_isolate());
       let state = state_rc.borrow();