Repository: denoland/deno_core
Pull request #324 (2023-11-11 14:22:07)

perf(core): try to fast path every realm case


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 1e36592..fbc24d2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -258,7 +258,7 @@ dependencies = [
 
 [[package]]
 name = "deno_core"
-version = "0.229.0"
+version = "0.230.0"
 dependencies = [
  "anyhow",
  "bencher",
@@ -296,7 +296,7 @@ dependencies = [
 
 [[package]]
 name = "deno_ops"
-version = "0.105.0"
+version = "0.106.0"
 dependencies = [
  "pretty_assertions",
  "prettyplease",
@@ -1187,7 +1187,7 @@ dependencies = [
 
 [[package]]
 name = "serde_v8"
-version = "0.138.0"
+version = "0.139.0"
 dependencies = [
  "bencher",
  "bytes",
diff --git a/Cargo.toml b/Cargo.toml
index ffd8f80..f02ddc3 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -17,9 +17,9 @@ repository = "https://github.com/denoland/deno_core"
 
 [workspace.dependencies]
 # Local dependencies
-deno_core = { version = "0.229.0", path = "./core" }
-deno_ops = { version = "0.105.0", path = "./ops" }
-serde_v8 = { version = "0.138.0", path = "./serde_v8" }
+deno_core = { version = "0.230.0", path = "./core" }
+deno_ops = { version = "0.106.0", path = "./ops" }
+serde_v8 = { version = "0.139.0", path = "./serde_v8" }
 
 v8 = { version = "0.81.0", default-features = false }
 deno_ast = { version = "0.31.2", features = ["transpiling"] }
diff --git a/core/Cargo.toml b/core/Cargo.toml
index 92a7b23..5bbadb2 100644
--- a/core/Cargo.toml
+++ b/core/Cargo.toml
@@ -2,7 +2,7 @@
 
 [package]
 name = "deno_core"
-version = "0.229.0"
+version = "0.230.0"
 authors.workspace = true
 edition.workspace = true
 license.workspace = true
diff --git a/core/lib.rs b/core/lib.rs
index b147020..e856a9e 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -97,6 +97,7 @@ pub use crate::modules::ModuleType;
 pub use crate::modules::NoopModuleLoader;
 pub use crate::modules::ResolutionKind;
 pub use crate::modules::StaticModuleLoader;
+pub use crate::modules::ValidateImportAttributesCb;
 pub use crate::normalize_path::normalize_path;
 pub use crate::ops::OpError;
 pub use crate::ops::OpId;
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 1125f0a..c9dbd03 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -5,7 +5,6 @@ use crate::error::throw_type_error;
 use crate::fast_string::FastString;
 use crate::modules::get_asserted_module_type_from_assertions;
 use crate::modules::parse_import_assertions;
-use crate::modules::validate_import_assertions;
 use crate::modules::ImportAssertionsKind;
 use crate::modules::ModuleCode;
 use crate::modules::ModuleError;
@@ -22,6 +21,7 @@ use crate::modules::RecursiveModuleLoad;
 use crate::modules::ResolutionKind;
 use crate::runtime::JsRealm;
 use crate::runtime::SnapshottedData;
+use crate::JsRuntime;
 use anyhow::Error;
 use futures::future::FutureExt;
 use futures::stream::FuturesUnordered;
@@ -505,7 +505,12 @@ impl ModuleMap {
 
       // FIXME(bartomieju): there are no stack frames if exception
       // is thrown here
-      validate_import_assertions(tc_scope, &assertions);
+      {
+        let state_rc = JsRuntime::state_from(tc_scope);
+        let state = state_rc.borrow();
+        (state.validate_import_attributes_cb)(tc_scope, &assertions);
+      }
+
       if tc_scope.has_caught() {
         let exception = tc_scope.exception().unwrap();
         let exception = v8::Global::new(tc_scope, exception);
@@ -821,20 +826,16 @@ impl ModuleMap {
       asserted_module_type,
       module_map_rc.clone(),
     );
-    module_map_rc
-      .borrow_mut()
-      .dynamic_import_map
-      .insert(load.id, resolver_handle);
 
-    let loader = module_map_rc.borrow().loader.clone();
+    let this: &mut ModuleMap = &mut module_map_rc.borrow_mut();
+    this.dynamic_import_map.insert(load.id, resolver_handle);
+
+    let loader = this.loader.clone();
     let resolve_result =
       loader.resolve(specifier, referrer, ResolutionKind::DynamicImport);
     let fut = match resolve_result {
       Ok(module_specifier) => {
-        if module_map_rc
-          .borrow()
-          .is_registered(module_specifier, asserted_module_type)
-        {
+        if this.is_registered(module_specifier, asserted_module_type) {
           async move { (load.id, Ok(load)) }.boxed_local()
         } else {
           async move { (load.id, load.prepare().await.map(|()| load)) }
@@ -843,10 +844,7 @@ impl ModuleMap {
       }
       Err(error) => async move { (load.id, Err(error)) }.boxed_local(),
     };
-    module_map_rc
-      .borrow_mut()
-      .preparing_dynamic_imports
-      .push(fut);
+    this.preparing_dynamic_imports.push(fut);
   }
 
   pub(crate) fn has_pending_dynamic_imports(&self) -> bool {
diff --git a/core/modules/mod.rs b/core/modules/mod.rs
index 2af22dd..64093f6 100644
--- a/core/modules/mod.rs
+++ b/core/modules/mod.rs
@@ -49,24 +49,36 @@ pub(crate) type ModuleLoadId = i32;
 pub type ModuleCode = FastString;
 pub type ModuleName = FastString;
 
+/// Callback to validate import attributes. If the validation fails and exception
+/// should be thrown using `scope.throw_exception()`.
+pub type ValidateImportAttributesCb =
+  Box<dyn Fn(&mut v8::HandleScope, &HashMap<String, String>)>;
+
 const SUPPORTED_TYPE_ASSERTIONS: &[&str] = &["json"];
 
-/// Throws V8 exception if assertions are invalid
-pub(crate) fn validate_import_assertions(
+/// Throws a `TypeError` if `type` attribute is not equal to "json". Allows
+/// all other attributes.
+pub(crate) fn validate_import_attributes(
   scope: &mut v8::HandleScope,
   assertions: &HashMap<String, String>,
 ) {
   for (key, value) in assertions {
-    if key == "type" && !SUPPORTED_TYPE_ASSERTIONS.contains(&value.as_str()) {
-      let message = v8::String::new(
-        scope,
-        &format!("\"{value}\" is not a valid module type."),
-      )
-      .unwrap();
-      let exception = v8::Exception::type_error(scope, message);
-      scope.throw_exception(exception);
-      return;
-    }
+    let msg = if key != "type" {
+      Some(format!("\"{key}\" attribute is not supported."))
+    } else if !SUPPORTED_TYPE_ASSERTIONS.contains(&value.as_str()) {
+      Some(format!("\"{value}\" is not a valid module type."))
+    } else {
+      None
+    };
+
+    let Some(msg) = msg else {
+      continue;
+    };
+
+    let message = v8::String::new(scope, &msg).unwrap();
+    let exception = v8::Exception::type_error(scope, message);
+    scope.throw_exception(exception);
+    return;
   }
 }
 
diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs
index 92835bb..81822ef 100644
--- a/core/runtime/bindings.rs
+++ b/core/runtime/bindings.rs
@@ -11,7 +11,6 @@ use crate::error::throw_type_error;
 use crate::error::JsStackFrame;
 use crate::modules::get_asserted_module_type_from_assertions;
 use crate::modules::parse_import_assertions;
-use crate::modules::validate_import_assertions;
 use crate::modules::ImportAssertionsKind;
 use crate::modules::ModuleMap;
 use crate::modules::ResolutionKind;
@@ -291,7 +290,12 @@ pub fn host_import_module_dynamically_callback<'s>(
 
   {
     let tc_scope = &mut v8::TryCatch::new(scope);
-    validate_import_assertions(tc_scope, &assertions);
+    {
+      let state_rc = JsRuntime::state_from(tc_scope);
+      let state = state_rc.borrow();
+      (state.validate_import_attributes_cb)(tc_scope, &assertions);
+    }
+
     if tc_scope.has_caught() {
       let e = tc_scope.exception().unwrap();
       resolver.reject(tc_scope, e);
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index f1b2357..35c1bb4 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -20,6 +20,7 @@ use crate::modules::ModuleCode;
 use crate::modules::ModuleId;
 use crate::modules::ModuleLoader;
 use crate::modules::ModuleMap;
+use crate::modules::ValidateImportAttributesCb;
 use crate::ops::*;
 use crate::ops_metrics::dispatch_metrics_async;
 use crate::ops_metrics::OpMetricsEvent;
@@ -236,6 +237,8 @@ pub struct JsRuntime {
   init_mode: InitMode,
   // Marks if this is considered the top-level runtime. Used only by inspector.
   is_main_runtime: bool,
+  /// Have we created additional realms? If so, we have to poison the fast path.
+  additional_realms: bool,
 }
 
 /// The runtime type used for snapshot creation.
@@ -317,6 +320,7 @@ pub struct JsRuntimeState {
   // flimsy. Try to poll it similarly to `pending_promise_rejections`.
   pub(crate) dispatched_exception: Option<v8::Global<v8::Value>>,
   pub(crate) inspector: Option<Rc<RefCell<JsRuntimeInspector>>>,
+  pub(crate) validate_import_attributes_cb: ValidateImportAttributesCb,
 }
 
 impl JsRuntimeState {
@@ -454,6 +458,11 @@ pub struct RuntimeOptions {
   /// An optional instance of `FeatureChecker`. If one is not provided, the
   /// default instance will be created that has no features enabled.
   pub feature_checker: Option<Arc<FeatureChecker>>,
+
+  /// A callback that can be used to validate import attributes received at
+  /// the import site. If not callback is provided, a default one is used. The
+  /// default callback only allows `"type"` attribute, with a value of `"json"`.
+  pub validate_import_attributes_cb: Option<ValidateImportAttributesCb>,
 }
 
 impl RuntimeOptions {
@@ -601,6 +610,10 @@ impl JsRuntime {
       // SAFETY: we just asserted that layout has non-0 size.
       unsafe { std::alloc::alloc(layout) as *mut _ };
 
+    let validate_import_attributes_cb = options
+      .validate_import_attributes_cb
+      .unwrap_or_else(|| Box::new(crate::modules::validate_import_attributes));
+
     let state_rc = Rc::new(RefCell::new(JsRuntimeState {
       dyn_module_evaluate_idle_counter: 0,
       has_tick_scheduled: false,
@@ -613,6 +626,7 @@ impl JsRuntime {
       // Some fields are initialized later after isolate is created
       inspector: None,
       known_realms: Vec::with_capacity(1),
+      validate_import_attributes_cb,
     }));
 
     let weak = Rc::downgrade(&state_rc);
@@ -781,6 +795,7 @@ impl JsRuntime {
       extensions: options.extensions,
       preserve_snapshotted_modules: options.preserve_snapshotted_modules,
       is_main_runtime: options.is_main,
+      additional_realms: false,
     };
 
     let realm = js_runtime.main_realm();
@@ -917,7 +932,10 @@ impl JsRuntime {
         self.inner.state.clone(),
         false,
       );
+
       let mut state = self.inner.state.borrow_mut();
+      // Poison the fast path for realms
+      self.additional_realms = true;
       state.known_realms.push(realm.clone());
       JsRealm::new(realm)
     };
@@ -1477,7 +1495,6 @@ impl JsRuntime {
     wait_for_inspector: bool,
   ) -> Poll<Result<(), Error>> {
     let has_inspector: bool;
-
     {
       let state = self.inner.state.borrow();
       has_inspector = state.inspector.is_some();
@@ -1512,30 +1529,28 @@ impl JsRuntime {
       loop {
         let mut has_evaluated = false;
 
-        let state = self.inner.state.borrow();
-        if state.known_realms.len() == 1 {
-          drop(state);
-          // Try and resolve as many dynamic imports in each realm as possible
-          // before moving to the next.
-          let realm = self.inner.main_realm.as_ref().unwrap();
-          loop {
-            let poll_imports =
-              realm.prepare_dyn_imports(&mut self.inner.v8_isolate, cx)?;
-            assert!(poll_imports.is_ready());
-
-            let poll_imports =
-              realm.poll_dyn_imports(&mut self.inner.v8_isolate, cx)?;
-            assert!(poll_imports.is_ready());
-
-            if realm.evaluate_dyn_imports(&mut self.inner.v8_isolate) {
-              has_evaluated = true;
-            } else {
-              break;
-            }
+        // Fast main realm poll
+        let main_realm = self.inner.main_realm.as_ref().unwrap();
+        loop {
+          let poll_imports =
+            main_realm.prepare_dyn_imports(&mut self.inner.v8_isolate, cx)?;
+          assert!(poll_imports.is_ready());
+
+          let poll_imports =
+            main_realm.poll_dyn_imports(&mut self.inner.v8_isolate, cx)?;
+          assert!(poll_imports.is_ready());
+
+          if main_realm.evaluate_dyn_imports(&mut self.inner.v8_isolate) {
+            has_evaluated = true;
+          } else {
+            break;
           }
-        } else {
-          // TODO(bartlomieju|mmastrac): Remove cloning in the runtime loop
-          let realms = state.known_realms.clone();
+        }
+
+        if self.additional_realms {
+          // TODO(bartlomieju|mmastrac): Remove cloning/to_vec in the runtime loop
+          let state = self.inner.state.borrow();
+          let realms = state.known_realms[1..].to_vec();
           drop(state);
           for inner_realm in realms {
             let realm = JsRealm::new(inner_realm);
@@ -1559,6 +1574,7 @@ impl JsRuntime {
             }
           }
         }
+
         if !has_evaluated {
           break;
         }
@@ -1585,14 +1601,16 @@ impl JsRuntime {
 
     // Top level module
     {
-      let state = self.inner.state.borrow();
-      if state.known_realms.len() == 1 {
-        drop(state);
-        let realm = self.inner.main_realm.as_ref().unwrap();
-        realm.evaluate_pending_module(&mut self.inner.v8_isolate);
-      } else {
+      self
+        .inner
+        .main_realm
+        .as_ref()
+        .unwrap()
+        .evaluate_pending_module(&mut self.inner.v8_isolate);
+      if self.additional_realms {
         // TODO(bartlomieju|mmastrac): Remove cloning in the runtime loop
-        let realms = state.known_realms.clone();
+        let state = self.inner.state.borrow();
+        let realms = state.known_realms[1..].to_vec();
         drop(state);
         for inner_realm in realms {
           let realm = JsRealm::new(inner_realm);
@@ -1601,7 +1619,19 @@ impl JsRuntime {
       }
     }
 
-    let pending_state = self.event_loop_pending_state();
+    // Get the pending state from the main realm, or all realms
+    let pending_state = if self.additional_realms {
+      let mut scope = v8::HandleScope::new(self.inner.v8_isolate.as_mut());
+      EventLoopPendingState::new(&mut scope, &mut self.inner.state.borrow_mut())
+    } else {
+      let mut scope = v8::HandleScope::new(self.inner.v8_isolate.as_mut());
+      EventLoopPendingState::new_from_main_realm(
+        &mut scope,
+        &mut self.inner.state.borrow_mut(),
+        &self.inner.main_realm.as_ref().unwrap().0,
+      )
+    };
+
     if !pending_state.is_pending() && !maybe_scheduling {
       if has_inspector {
         let inspector = self.inspector();
@@ -1626,8 +1656,6 @@ impl JsRuntime {
       return Poll::Ready(Ok(()));
     }
 
-    let state = self.inner.state.borrow();
-
     // Check if more async ops have been dispatched
     // during this turn of event loop.
     // If there are any pending background tasks, we also wake the runtime to
@@ -1635,23 +1663,25 @@ impl JsRuntime {
     // TODO(andreubotella) The event loop will spin as long as there are pending
     // background tasks. We should look into having V8 notify us when a
     // background task is done.
-    if pending_state.has_pending_background_tasks
-      || pending_state.has_tick_scheduled
-      || maybe_scheduling
-    {
-      state.op_state.borrow().waker.wake();
-    }
-
-    // If ops were dispatched we may have progress on pending modules that we should re-check
-    if (pending_state.has_pending_module_evaluation
-      || pending_state.has_pending_dyn_module_evaluation)
-      && dispatched_ops
+    #[allow(clippy::suspicious_else_formatting, clippy::if_same_then_else)]
     {
-      state.op_state.borrow().waker.wake();
+      if pending_state.has_pending_background_tasks
+        || pending_state.has_tick_scheduled
+        || maybe_scheduling
+      {
+        let state = self.inner.state.borrow();
+        state.op_state.borrow().waker.wake();
+      } else
+      // If ops were dispatched we may have progress on pending modules that we should re-check
+      if (pending_state.has_pending_module_evaluation
+        || pending_state.has_pending_dyn_module_evaluation)
+        && dispatched_ops
+      {
+        let state = self.inner.state.borrow();
+        state.op_state.borrow().waker.wake();
+      }
     }
 
-    drop(state);
-
     if pending_state.has_pending_module_evaluation {
       if pending_state.has_pending_refed_ops
         || pending_state.has_pending_dyn_imports
@@ -1700,11 +1730,6 @@ impl JsRuntime {
 
     Poll::Pending
   }
-
-  fn event_loop_pending_state(&mut self) -> EventLoopPendingState {
-    let mut scope = v8::HandleScope::new(self.inner.v8_isolate.as_mut());
-    EventLoopPendingState::new(&mut scope, &mut self.inner.state.borrow_mut())
-  }
 }
 
 fn find_and_report_stalled_level_await_in_any_realm(
@@ -1822,6 +1847,27 @@ pub(crate) struct EventLoopPendingState {
   has_tick_scheduled: bool,
 }
 impl EventLoopPendingState {
+  pub fn new_from_main_realm(
+    scope: &mut v8::HandleScope<()>,
+    state: &mut JsRuntimeState,
+    realm: &JsRealmInner,
+  ) -> Self {
+    let num_unrefed_ops = realm.num_unrefed_ops();
+    let num_pending_ops = realm.num_pending_ops();
+    let has_pending_dyn_imports = realm.has_pending_dyn_imports();
+    let has_pending_dyn_module_evaluation =
+      realm.has_pending_dyn_module_evaluation();
+    let has_pending_module_evaluation = realm.has_pending_module_evaluation();
+    EventLoopPendingState {
+      has_pending_refed_ops: num_pending_ops > num_unrefed_ops,
+      has_pending_dyn_imports,
+      has_pending_dyn_module_evaluation,
+      has_pending_module_evaluation,
+      has_pending_background_tasks: scope.has_pending_background_tasks(),
+      has_tick_scheduled: state.has_tick_scheduled,
+    }
+  }
+
   pub fn new(
     scope: &mut v8::HandleScope<()>,
     state: &mut JsRuntimeState,
@@ -1979,82 +2025,102 @@ impl JsRuntime {
     // Handle responses for each realm.
     let state = self.inner.state.clone();
     let isolate = &mut self.inner.v8_isolate;
-    let realm_count = state.borrow().known_realms.len();
-    let mut dispatched_ops = false;
 
-    for realm_idx in 0..realm_count {
-      let realm = state.borrow().known_realms.get(realm_idx).unwrap().clone();
-      let context_state = realm.state();
-      let mut context_state = context_state.borrow_mut();
-      let scope = &mut realm.handle_scope(isolate);
-
-      // We return async responses to JS in unbounded batches (may change),
-      // each batch is a flat vector of tuples:
-      // `[promise_id1, op_result1, promise_id2, op_result2, ...]`
-      // promise_id is a simple integer, op_result is an ops::OpResult
-      // which contains a value OR an error, encoded as a tuple.
-      // This batch is received in JS via the special `arguments` variable
-      // and then each tuple is used to resolve or reject promises
-      //
-      // This can handle 15 promises futures in a single batch without heap
-      // allocations.
-      let mut args: SmallVec<[v8::Local<v8::Value>; 32]> =
-        SmallVec::with_capacity(32);
+    let mut dispatched_ops = Self::do_js_event_loop_tick_realm(
+      cx,
+      isolate,
+      &state,
+      &self.inner.main_realm.as_ref().unwrap().0,
+    )?;
+    if self.additional_realms {
+      let realm_count = state.borrow().known_realms.len();
+      for realm_idx in 1..realm_count {
+        let realm = state.borrow().known_realms.get(realm_idx).unwrap().clone();
+        dispatched_ops |=
+          Self::do_js_event_loop_tick_realm(cx, isolate, &state, &realm)?;
+      }
+    }
 
-      loop {
-        let Poll::Ready(item) = context_state.pending_ops.poll_join_next(cx) else {
-          break;
-        };
-        // TODO(mmastrac): If this task is really errored, things could be pretty bad
-        let PendingOp(promise_id, op_id, resp, metrics_event) = item.unwrap();
-        context_state.unrefed_ops.remove(&promise_id);
-        dispatched_ops |= true;
-        args.push(v8::Integer::new(scope, promise_id).into());
-        let was_error = matches!(resp, OpResult::Err(_));
-        let res = resp.to_v8(scope);
-        if metrics_event {
-          if res.is_ok() && !was_error {
-            dispatch_metrics_async(
-              &context_state.op_ctxs[op_id as usize],
-              OpMetricsEvent::CompletedAsync,
-            );
-          } else {
-            dispatch_metrics_async(
-              &context_state.op_ctxs[op_id as usize],
-              OpMetricsEvent::ErrorAsync,
-            );
-          }
+    Ok(dispatched_ops)
+  }
+
+  fn do_js_event_loop_tick_realm(
+    cx: &mut Context,
+    isolate: &mut ManuallyDrop<v8::OwnedIsolate>,
+    state: &Rc<RefCell<JsRuntimeState>>,
+    realm: &JsRealmInner,
+  ) -> Result<bool, Error> {
+    let context_state = realm.state();
+    let mut context_state = context_state.borrow_mut();
+    let scope = &mut realm.handle_scope(isolate);
+    let mut dispatched_ops = false;
+
+    // We return async responses to JS in unbounded batches (may change),
+    // each batch is a flat vector of tuples:
+    // `[promise_id1, op_result1, promise_id2, op_result2, ...]`
+    // promise_id is a simple integer, op_result is an ops::OpResult
+    // which contains a value OR an error, encoded as a tuple.
+    // This batch is received in JS via the special `arguments` variable
+    // and then each tuple is used to resolve or reject promises
+    //
+    // This can handle 15 promises futures in a single batch without heap
+    // allocations.
+    let mut args: SmallVec<[v8::Local<v8::Value>; 32]> =
+      SmallVec::with_capacity(32);
+
+    loop {
+      let Poll::Ready(item) = context_state.pending_ops.poll_join_next(cx) else {
+        break;
+      };
+      // TODO(mmastrac): If this task is really errored, things could be pretty bad
+      let PendingOp(promise_id, op_id, resp, metrics_event) = item.unwrap();
+      context_state.unrefed_ops.remove(&promise_id);
+      dispatched_ops |= true;
+      args.push(v8::Integer::new(scope, promise_id).into());
+      let was_error = matches!(resp, OpResult::Err(_));
+      let res = resp.to_v8(scope);
+      if metrics_event {
+        if res.is_ok() && !was_error {
+          dispatch_metrics_async(
+            &context_state.op_ctxs[op_id as usize],
+            OpMetricsEvent::CompletedAsync,
+          );
+        } else {
+          dispatch_metrics_async(
+            &context_state.op_ctxs[op_id as usize],
+            OpMetricsEvent::ErrorAsync,
+          );
         }
-        args.push(match res {
-          Ok(v) => v,
-          Err(e) => OpResult::Err(OpError::new(&|_| "TypeError", e.into()))
-            .to_v8(scope)
-            .unwrap(),
-        });
       }
+      args.push(match res {
+        Ok(v) => v,
+        Err(e) => OpResult::Err(OpError::new(&|_| "TypeError", e.into()))
+          .to_v8(scope)
+          .unwrap(),
+      });
+    }
 
-      let has_tick_scheduled = self.inner.state.borrow().has_tick_scheduled;
-      dispatched_ops |= has_tick_scheduled;
-      let has_tick_scheduled = v8::Boolean::new(scope, has_tick_scheduled);
-      args.push(has_tick_scheduled.into());
-
-      let js_event_loop_tick_cb_handle =
-        context_state.js_event_loop_tick_cb.clone().unwrap();
-      let tc_scope = &mut v8::TryCatch::new(scope);
-      let js_event_loop_tick_cb = js_event_loop_tick_cb_handle.open(tc_scope);
-      let this = v8::undefined(tc_scope).into();
-      drop(context_state);
-      js_event_loop_tick_cb.call(tc_scope, this, args.as_slice());
-
-      if let Some(exception) = tc_scope.exception() {
-        // TODO(@andreubotella): Returning here can cause async ops in other
-        // realms to never resolve.
-        return exception_to_err_result(tc_scope, exception, false);
-      }
+    let has_tick_scheduled = state.borrow().has_tick_scheduled;
+    dispatched_ops |= has_tick_scheduled;
+    let has_tick_scheduled = v8::Boolean::new(scope, has_tick_scheduled);
+    args.push(has_tick_scheduled.into());
 
-      if tc_scope.has_terminated() || tc_scope.is_execution_terminating() {
-        return Ok(false);
-      }
+    let js_event_loop_tick_cb_handle =
+      context_state.js_event_loop_tick_cb.clone().unwrap();
+    let tc_scope = &mut v8::TryCatch::new(scope);
+    let js_event_loop_tick_cb = js_event_loop_tick_cb_handle.open(tc_scope);
+    let this = v8::undefined(tc_scope).into();
+    drop(context_state);
+    js_event_loop_tick_cb.call(tc_scope, this, args.as_slice());
+
+    if let Some(exception) = tc_scope.exception() {
+      // TODO(@andreubotella): Returning here can cause async ops in other
+      // realms to never resolve.
+      return exception_to_err_result(tc_scope, exception, false);
+    }
+
+    if tc_scope.has_terminated() || tc_scope.is_execution_terminating() {
+      return Ok(false);
     }
 
     Ok(dispatched_ops)
diff --git a/ops/Cargo.toml b/ops/Cargo.toml
index 12895b5..9e00780 100644
--- a/ops/Cargo.toml
+++ b/ops/Cargo.toml
@@ -2,7 +2,7 @@
 
 [package]
 name = "deno_ops"
-version = "0.105.0"
+version = "0.106.0"
 authors.workspace = true
 edition.workspace = true
 license.workspace = true
diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml
index 1e9eaab..001e5d4 100644
--- a/serde_v8/Cargo.toml
+++ b/serde_v8/Cargo.toml
@@ -2,7 +2,7 @@
 
 [package]
 name = "serde_v8"
-version = "0.138.0"
+version = "0.139.0"
 authors.workspace = true
 edition.workspace = true
 license.workspace = true


Test patch:
diff --git a/core/modules/tests.rs b/core/modules/tests.rs
index e72ce80..531f8eb 100644
--- a/core/modules/tests.rs
+++ b/core/modules/tests.rs
@@ -1,5 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 use crate::ascii_str;
+use crate::error::exception_to_err_result;
 use crate::modules::loaders::CountingModuleLoader;
 use crate::modules::loaders::ModuleLoadEventCounts;
 use crate::resolve_import;
@@ -495,6 +496,125 @@ fn test_json_module() {
   futures::executor::block_on(receiver).unwrap().unwrap();
 }
 
+#[test]
+fn test_validate_import_attributes() {
+  fn validate_import_attrs(
+    scope: &mut v8::HandleScope,
+    _attrs: &HashMap<String, String>,
+  ) {
+    let msg = v8::String::new(scope, "boom!").unwrap();
+    let ex = v8::Exception::type_error(scope, msg);
+    scope.throw_exception(ex);
+  }
+
+  let loader = Rc::new(CountingModuleLoader::new(StaticModuleLoader::new([])));
+  let mut runtime = JsRuntime::new(RuntimeOptions {
+    module_loader: Some(loader.clone()),
+    validate_import_attributes_cb: Some(Box::new(validate_import_attrs)),
+    ..Default::default()
+  });
+
+  let module_map_rc = runtime.module_map().clone();
+
+  {
+    let scope = &mut runtime.handle_scope();
+    let mut module_map = module_map_rc.borrow_mut();
+    let specifier_a = ascii_str!("file:///a.js");
+    let module_err = module_map
+      .new_es_module(
+        scope,
+        true,
+        specifier_a,
+        ascii_str!(
+          r#"
+          import jsonData from './c.json' with {foo: "bar"};
+        "#
+        ),
+        false,
+      )
+      .unwrap_err();
+
+    let ModuleError::Exception(exc) = module_err else {
+      unreachable!();
+    };
+    let exception = v8::Local::new(scope, exc);
+    let err =
+      exception_to_err_result::<()>(scope, exception, false).unwrap_err();
+    assert_eq!(err.to_string(), "Uncaught TypeError: boom!");
+  }
+}
+
+#[test]
+fn test_validate_import_attributes_default() {
+  let loader = Rc::new(CountingModuleLoader::new(StaticModuleLoader::new([])));
+  let mut runtime = JsRuntime::new(RuntimeOptions {
+    module_loader: Some(loader.clone()),
+    ..Default::default()
+  });
+
+  let module_map_rc = runtime.module_map().clone();
+
+  {
+    let scope = &mut runtime.handle_scope();
+    let mut module_map = module_map_rc.borrow_mut();
+    let specifier_a = ascii_str!("file:///a.js");
+    let module_err = module_map
+      .new_es_module(
+        scope,
+        true,
+        specifier_a,
+        ascii_str!(
+          r#"
+          import jsonData from './c.json' with {foo: "bar"};
+        "#
+        ),
+        false,
+      )
+      .unwrap_err();
+
+    let ModuleError::Exception(exc) = module_err else {
+      unreachable!();
+    };
+    let exception = v8::Local::new(scope, exc);
+    let err =
+      exception_to_err_result::<()>(scope, exception, false).unwrap_err();
+    assert_eq!(
+      err.to_string(),
+      "Uncaught TypeError: \"foo\" attribute is not supported."
+    );
+  }
+
+  {
+    let scope = &mut runtime.handle_scope();
+    let mut module_map = module_map_rc.borrow_mut();
+    let specifier_a = ascii_str!("file:///a.js");
+    let module_err = module_map
+      .new_es_module(
+        scope,
+        true,
+        specifier_a,
+        ascii_str!(
+          r#"
+          import jsonData from './c.json' with {type: "bar"};
+        "#
+        ),
+        false,
+      )
+      .unwrap_err();
+
+    let ModuleError::Exception(exc) = module_err else {
+      unreachable!();
+    };
+    let exception = v8::Local::new(scope, exc);
+    let err =
+      exception_to_err_result::<()>(scope, exception, false).unwrap_err();
+    assert_eq!(
+      err.to_string(),
+      "Uncaught TypeError: \"bar\" is not a valid module type."
+    );
+  }
+}
+
 #[tokio::test]
 async fn dyn_import_err() {
   let loader = Rc::new(CountingModuleLoader::new(StaticModuleLoader::new([])));