refactor(core): store v8::Global<v8::Context> in an Rc (#18749)

Alternative to https://github.com/denoland/deno/pull/18726.

This was suggested by @piscisaureus. It's a bit ugly, but it does the
work and makes cloning `JsRealm` very cheap, while not requiring 
invasive changes.

Also managed to remove some vector and `v8::Global` clones which yields
about 5% improvement in the "async_ops_deferred.js" benchmark.

This PR:
```
time 1689 ms rate 592066
time 1722 ms rate 580720
time 1629 ms rate 613873
time 1578 ms rate 633713
time 1585 ms rate 630914
time 1574 ms rate 635324
```

`main` branch:
```
time 1687 ms rate 592768
time 1676 ms rate 596658
time 1651 ms rate 605693
time 1652 ms rate 605326
time 1638 ms rate 610500
```

core/error.rs

@@ -210,15 +210,18 @@ impl JsStackFrame {
     // V8's column numbers are 0-based, we want 1-based.
     let c = message.get_start_column() as i64 + 1;
     let state_rc = JsRuntime::state(scope);
-    let state = &mut *state_rc.borrow_mut();
-    if let Some(source_map_getter) = &state.source_map_getter {
-      let (f, l, c) = apply_source_map(
-        f,
-        l,
-        c,
-        &mut state.source_map_cache,
-        source_map_getter.as_ref(),
-      );
+    let (getter, cache) = {
+      let state = state_rc.borrow();
+      (
+        state.source_map_getter.clone(),
+        state.source_map_cache.clone(),
+      )
+    };
+
+    if let Some(source_map_getter) = getter {
+      let mut cache = cache.borrow_mut();
+      let (f, l, c) =
+        apply_source_map(f, l, c, &mut cache, &**source_map_getter);
       Some(JsStackFrame::from_location(Some(f), Some(l), Some(c)))
     } else {
       Some(JsStackFrame::from_location(Some(f), Some(l), Some(c)))
@@ -280,8 +283,15 @@ impl JsError {
     }
     {
       let state_rc = JsRuntime::state(scope);
-      let state = &mut *state_rc.borrow_mut();
-      if let Some(source_map_getter) = &state.source_map_getter {
+      let (getter, cache) = {
+        let state = state_rc.borrow();
+        (
+          state.source_map_getter.clone(),
+          state.source_map_cache.clone(),
+        )
+      };
+      if let Some(source_map_getter) = getter {
+        let mut cache = cache.borrow_mut();
         for (i, frame) in frames.iter().enumerate() {
           if let (Some(file_name), Some(line_number)) =
             (&frame.file_name, frame.line_number)
@@ -290,8 +300,8 @@ impl JsError {
               source_line = get_source_line(
                 file_name,
                 line_number,
-                &mut state.source_map_cache,
-                source_map_getter.as_ref(),
+                &mut cache,
+                &**source_map_getter,
               );
               source_line_frame_index = Some(i);
               break;
@@ -405,8 +415,16 @@ impl JsError {
       }
       {
         let state_rc = JsRuntime::state(scope);
-        let state = &mut *state_rc.borrow_mut();
-        if let Some(source_map_getter) = &state.source_map_getter {
+        let (getter, cache) = {
+          let state = state_rc.borrow();
+          (
+            state.source_map_getter.clone(),
+            state.source_map_cache.clone(),
+          )
+        };
+        if let Some(source_map_getter) = getter {
+          let mut cache = cache.borrow_mut();
+
           for (i, frame) in frames.iter().enumerate() {
             if let (Some(file_name), Some(line_number)) =
               (&frame.file_name, frame.line_number)
@@ -415,8 +433,8 @@ impl JsError {
                 source_line = get_source_line(
                   file_name,
                   line_number,
-                  &mut state.source_map_cache,
-                  source_map_getter.as_ref(),
+                  &mut cache,
+                  &**source_map_getter,
                 );
                 source_line_frame_index = Some(i);
                 break;

core/ops_builtin_v8.rs

@@ -8,7 +8,7 @@ use crate::error::JsError;
 use crate::ops_builtin::WasmStreamingResource;
 use crate::resolve_url;
 use crate::serde_v8::from_v8;
-use crate::source_map::apply_source_map as apply_source_map_;
+use crate::source_map::apply_source_map;
 use crate::JsRealm;
 use crate::JsRuntime;
 use crate::ZeroCopyBuf;
@@ -17,6 +17,7 @@ use deno_ops::op;
 use serde::Deserialize;
 use serde::Serialize;
 use std::cell::RefCell;
+use std::rc::Rc;
 use v8::ValueDeserializerHelper;
 use v8::ValueSerializerHelper;
 
@@ -59,8 +60,8 @@ fn op_set_promise_reject_callback<'a>(
   let old = context_state_rc
     .borrow_mut()
     .js_promise_reject_cb
-    .replace(cb);
-  let old = old.map(|v| v8::Local::new(scope, v));
+    .replace(Rc::new(cb));
+  let old = old.map(|v| v8::Local::new(scope, &*v));
   Ok(old.map(|v| from_v8(scope, v.into()).unwrap()))
 }
 
@@ -633,7 +634,7 @@ fn op_set_wasm_streaming_callback(
   if context_state.js_wasm_streaming_cb.is_some() {
     return Err(type_error("op_set_wasm_streaming_callback already called"));
   }
-  context_state.js_wasm_streaming_cb = Some(cb);
+  context_state.js_wasm_streaming_cb = Some(Rc::new(cb));
 
   scope.set_wasm_streaming_callback(|scope, arg, wasm_streaming| {
     let (cb_handle, streaming_rid) = {
@@ -755,15 +756,23 @@ fn op_apply_source_map(
   location: Location,
 ) -> Result<Location, Error> {
   let state_rc = JsRuntime::state(scope);
-  let state = &mut *state_rc.borrow_mut();
-  if let Some(source_map_getter) = &state.source_map_getter {
+  let (getter, cache) = {
+    let state = state_rc.borrow();
+    (
+      state.source_map_getter.clone(),
+      state.source_map_cache.clone(),
+    )
+  };
+
+  if let Some(source_map_getter) = getter {
+    let mut cache = cache.borrow_mut();
     let mut location = location;
-    let (f, l, c) = apply_source_map_(
+    let (f, l, c) = apply_source_map(
       location.file_name,
       location.line_number.into(),
       location.column_number.into(),
-      &mut state.source_map_cache,
-      source_map_getter.as_ref(),
+      &mut cache,
+      &**source_map_getter,
     );
     location.file_name = f;
     location.line_number = l as u32;
@@ -788,14 +797,14 @@ fn op_set_format_exception_callback<'a>(
   let old = context_state_rc
     .borrow_mut()
     .js_format_exception_cb
-    .replace(cb);
-  let old = old.map(|v| v8::Local::new(scope, v));
+    .replace(Rc::new(cb));
+  let old = old.map(|v| v8::Local::new(scope, &*v));
   Ok(old.map(|v| from_v8(scope, v.into()).unwrap()))
 }
 
 #[op(v8)]
 fn op_event_loop_has_more_work(scope: &mut v8::HandleScope) -> bool {
-  JsRuntime::event_loop_pending_state_from_isolate(scope).is_pending()
+  JsRuntime::event_loop_pending_state_from_scope(scope).is_pending()
 }
 
 #[op(v8)]

core/realm.rs

@@ -8,21 +8,43 @@ use anyhow::Error;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::collections::HashSet;
+use std::hash::BuildHasherDefault;
+use std::hash::Hasher;
+use std::marker::PhantomData;
 use std::option::Option;
 use std::rc::Rc;
 use v8::HandleScope;
 use v8::Local;
 
+// Hasher used for `unrefed_ops`. Since these are rolling i32, there's no
+// need to actually hash them.
+#[derive(Default)]
+pub(crate) struct IdentityHasher(u64, PhantomData<i32>);
+
+impl Hasher for IdentityHasher {
+  fn write_i32(&mut self, i: i32) {
+    self.0 = i as u64;
+  }
+
+  fn finish(&self) -> u64 {
+    self.0
+  }
+
+  fn write(&mut self, _bytes: &[u8]) {
+    unreachable!()
+  }
+}
+
 #[derive(Default)]
 pub(crate) struct ContextState {
-  pub(crate) js_event_loop_tick_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_build_custom_error_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) js_event_loop_tick_cb: Option<Rc<v8::Global<v8::Function>>>,
+  pub(crate) js_build_custom_error_cb: Option<Rc<v8::Global<v8::Function>>>,
+  pub(crate) js_promise_reject_cb: Option<Rc<v8::Global<v8::Function>>>,
+  pub(crate) js_format_exception_cb: Option<Rc<v8::Global<v8::Function>>>,
+  pub(crate) js_wasm_streaming_cb: Option<Rc<v8::Global<v8::Function>>>,
   pub(crate) pending_promise_rejections:
     HashMap<v8::Global<v8::Promise>, v8::Global<v8::Value>>,
-  pub(crate) unrefed_ops: HashSet<i32>,
+  pub(crate) unrefed_ops: HashSet<i32, BuildHasherDefault<IdentityHasher>>,
   // We don't explicitly re-read this prop but need the slice to live alongside
   // the context
   pub(crate) op_ctxs: Box<[OpCtx]>,
@@ -73,16 +95,18 @@ pub(crate) struct ContextState {
 /// keep the underlying V8 context alive even if it would have otherwise been
 /// garbage collected.
 #[derive(Clone)]
-pub struct JsRealm(v8::Global<v8::Context>);
+pub struct JsRealm(Rc<v8::Global<v8::Context>>);
 impl JsRealm {
   pub fn new(context: v8::Global<v8::Context>) -> Self {
-    JsRealm(context)
+    JsRealm(Rc::new(context))
   }
 
+  #[inline(always)]
   pub fn context(&self) -> &v8::Global<v8::Context> {
     &self.0
   }
 
+  #[inline(always)]
   pub(crate) fn state(
     &self,
     isolate: &mut v8::Isolate,
@@ -95,6 +119,7 @@ impl JsRealm {
       .clone()
   }
 
+  #[inline(always)]
   pub(crate) fn state_from_scope(
     scope: &mut v8::HandleScope,
   ) -> Rc<RefCell<ContextState>> {
@@ -106,11 +131,12 @@ impl JsRealm {
   }
 
   /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
+  #[inline(always)]
   pub fn handle_scope<'s>(
     &self,
     isolate: &'s mut v8::Isolate,
   ) -> v8::HandleScope<'s> {
-    v8::HandleScope::with_context(isolate, &self.0)
+    v8::HandleScope::with_context(isolate, &*self.0)
   }
 
   /// For info on the [`v8::Isolate`] parameter, check [`JsRealm#panics`].
@@ -212,12 +238,36 @@ impl JsRealm {
   }
 
   // TODO(andreubotella): `mod_evaluate`, `load_main_module`, `load_side_module`
+}
+
+pub struct JsRealmLocal<'s>(v8::Local<'s, v8::Context>);
+impl<'s> JsRealmLocal<'s> {
+  pub fn new(context: v8::Local<'s, v8::Context>) -> Self {
+    JsRealmLocal(context)
+  }
+
+  #[inline(always)]
+  pub fn context(&self) -> v8::Local<v8::Context> {
+    self.0
+  }
+
+  #[inline(always)]
+  pub(crate) fn state(
+    &self,
+    isolate: &mut v8::Isolate,
+  ) -> Rc<RefCell<ContextState>> {
+    self
+      .context()
+      .get_slot::<Rc<RefCell<ContextState>>>(isolate)
+      .unwrap()
+      .clone()
+  }
 
   pub(crate) fn check_promise_rejections(
     &self,
-    isolate: &mut v8::Isolate,
+    scope: &mut v8::HandleScope,
   ) -> Result<(), Error> {
-    let context_state_rc = self.state(isolate);
+    let context_state_rc = self.state(scope);
     let mut context_state = context_state_rc.borrow_mut();
 
     if context_state.pending_promise_rejections.is_empty() {
@@ -238,7 +288,6 @@ impl JsRealm {
       .unwrap();
     drop(context_state);
 
-    let scope = &mut self.handle_scope(isolate);
     let exception = v8::Local::new(scope, handle);
     exception_to_err_result(scope, exception, true)
   }

core/runtime.rs

@@ -20,6 +20,7 @@ use crate::op_void_sync;
 use crate::ops::*;
 use crate::realm::ContextState;
 use crate::realm::JsRealm;
+use crate::realm::JsRealmLocal;
 use crate::snapshot_util;
 use crate::source_map::SourceMapCache;
 use crate::source_map::SourceMapGetter;
@@ -164,8 +165,8 @@ pub struct JsRuntimeState {
   /// A counter used to delay our dynamic import deadlock detection by one spin
   /// of the event loop.
   dyn_module_evaluate_idle_counter: u32,
-  pub(crate) source_map_getter: Option<Box<dyn SourceMapGetter>>,
-  pub(crate) source_map_cache: SourceMapCache,
+  pub(crate) source_map_getter: Option<Rc<Box<dyn SourceMapGetter>>>,
+  pub(crate) source_map_cache: Rc<RefCell<SourceMapCache>>,
   pub(crate) pending_ops: FuturesUnordered<PendingOpFuture>,
   pub(crate) have_unpolled_ops: bool,
   pub(crate) op_state: Rc<RefCell<OpState>>,
@@ -341,7 +342,7 @@ impl JsRuntime {
       pending_mod_evaluate: None,
       dyn_module_evaluate_idle_counter: 0,
       has_tick_scheduled: false,
-      source_map_getter: options.source_map_getter,
+      source_map_getter: options.source_map_getter.map(Rc::new),
       source_map_cache: Default::default(),
       pending_ops: FuturesUnordered::new(),
       shared_array_buffer_store: options.shared_array_buffer_store,
@@ -871,10 +872,12 @@ impl JsRuntime {
     // Put global handles in the realm's ContextState
     let state_rc = realm.state(self.v8_isolate());
     let mut state = state_rc.borrow_mut();
-    state.js_event_loop_tick_cb.replace(event_loop_tick_cb);
+    state
+      .js_event_loop_tick_cb
+      .replace(Rc::new(event_loop_tick_cb));
     state
       .js_build_custom_error_cb
-      .replace(build_custom_error_cb);
+      .replace(Rc::new(build_custom_error_cb));
   }
 
   /// Returns the runtime's op state, which can be used to maintain ops
@@ -972,27 +975,26 @@ impl JsRuntime {
       Self::drop_state_and_module_map(v8_isolate);
     }
 
-    self.state.borrow_mut().global_realm.take();
-
     // Drop other v8::Global handles before snapshotting
     {
       for weak_context in &self.state.clone().borrow().known_realms {
-        let v8_isolate = self.v8_isolate();
-        if let Some(context) = weak_context.to_global(v8_isolate) {
-          let realm = JsRealm::new(context.clone());
-          let realm_state_rc = realm.state(v8_isolate);
+        let scope = &mut self.handle_scope();
+        if let Some(context) = weak_context.to_local(scope) {
+          let realm = JsRealmLocal::new(context);
+          let realm_state_rc = realm.state(scope);
           let mut realm_state = realm_state_rc.borrow_mut();
           std::mem::take(&mut realm_state.js_event_loop_tick_cb);
           std::mem::take(&mut realm_state.js_build_custom_error_cb);
           std::mem::take(&mut realm_state.js_promise_reject_cb);
           std::mem::take(&mut realm_state.js_format_exception_cb);
           std::mem::take(&mut realm_state.js_wasm_streaming_cb);
-          context.open(v8_isolate).clear_all_slots(v8_isolate);
+          context.clear_all_slots(scope);
         }
       }
 
       let mut state = self.state.borrow_mut();
       state.known_realms.clear();
+      state.global_realm.take();
     }
 
     let snapshot_creator = self.v8_isolate.take().unwrap();
@@ -1335,21 +1337,26 @@ impl JsRuntime {
   }
 
   fn event_loop_pending_state(&mut self) -> EventLoopPendingState {
+    let isolate = self.v8_isolate.as_mut().unwrap();
+    let mut scope = v8::HandleScope::new(isolate);
     EventLoopPendingState::new(
-      self.v8_isolate.as_mut().unwrap(),
+      &mut scope,
       &mut self.state.borrow_mut(),
       &self.module_map.as_ref().unwrap().borrow(),
     )
   }
 
-  pub(crate) fn event_loop_pending_state_from_isolate(
-    isolate: &mut v8::Isolate,
+  pub(crate) fn event_loop_pending_state_from_scope(
+    scope: &mut v8::HandleScope,
   ) -> EventLoopPendingState {
-    EventLoopPendingState::new(
-      isolate,
-      &mut Self::state(isolate).borrow_mut(),
-      &Self::module_map(isolate).borrow(),
-    )
+    let state = Self::state(scope);
+    let module_map = Self::module_map(scope);
+    let state = EventLoopPendingState::new(
+      scope,
+      &mut state.borrow_mut(),
+      &module_map.borrow(),
+    );
+    state
   }
 }
 
@@ -1416,15 +1423,21 @@ pub(crate) struct EventLoopPendingState {
 }
 impl EventLoopPendingState {
   pub fn new(
-    isolate: &mut v8::Isolate,
+    scope: &mut v8::HandleScope<()>,
     state: &mut JsRuntimeState,
     module_map: &ModuleMap,
   ) -> EventLoopPendingState {
     let mut num_unrefed_ops = 0;
+
+    if state.known_realms.len() == 1 {
+      let realm = state.global_realm.as_ref().unwrap();
+      num_unrefed_ops += realm.state(scope).borrow().unrefed_ops.len();
+    } else {
       for weak_context in &state.known_realms {
-      if let Some(context) = weak_context.to_global(isolate) {
-        let realm = JsRealm::new(context);
-        num_unrefed_ops += realm.state(isolate).borrow().unrefed_ops.len();
+        if let Some(context) = weak_context.to_local(scope) {
+          let realm = JsRealmLocal::new(context);
+          num_unrefed_ops += realm.state(scope).borrow().unrefed_ops.len();
+        }
       }
     }
 
@@ -1435,7 +1448,7 @@ impl EventLoopPendingState {
         .pending_dyn_mod_evaluate
         .is_empty(),
       has_pending_module_evaluation: state.pending_mod_evaluate.is_some(),
-      has_pending_background_tasks: isolate.has_pending_background_tasks(),
+      has_pending_background_tasks: scope.has_pending_background_tasks(),
       has_tick_scheduled: state.has_tick_scheduled,
     }
   }
@@ -2176,11 +2189,12 @@ impl JsRuntime {
   }
 
   fn check_promise_rejections(&mut self) -> Result<(), Error> {
-    let known_realms = self.state.borrow().known_realms.clone();
-    let isolate = self.v8_isolate();
-    for weak_context in known_realms {
-      if let Some(context) = weak_context.to_global(isolate) {
-        JsRealm::new(context).check_promise_rejections(isolate)?;
+    let state = self.state.clone();
+    let scope = &mut self.handle_scope();
+    let state = state.borrow();
+    for weak_context in &state.known_realms {
+      if let Some(context) = weak_context.to_local(scope) {
+        JsRealmLocal::new(context).check_promise_rejections(scope)?;
       }
     }
     Ok(())