diff --git a/moli-renderer-v8/src/common_runtime.rs b/moli-renderer-v8/src/common_runtime.rs index 523eafde66..1cb62d3d0d 100644 --- a/moli-renderer-v8/src/common_runtime.rs +++ b/moli-renderer-v8/src/common_runtime.rs @@ -111,46 +111,75 @@ impl CommonRuntimeBinding { ); } - pub(crate) fn from_scope(scope: &mut v8::PinScope<'_, '_>) -> Option> { - let binding = scope.get_current_context().get_slot::()?; - binding.validate_context(scope)?; - Some(binding) + /// Resolve the current Context and its owner once for immediate native + /// dispatch. This result is not permission to invoke a later callback: + /// invocation must revalidate the binding through `callback_execution_context`. + pub(crate) fn from_scope( + scope: &mut v8::PinScope<'_, '_>, + ) -> Option<(Rc, RuntimeExecutionContext)> { + let context = scope.get_current_context(); + let binding = context.get_slot::()?; + let owner = binding.resolve_owner(context)?; + Some((binding, owner)) } pub(crate) fn runtime(&self) -> Option> { self.common.upgrade() } - fn validate_context(&self, scope: &mut v8::PinScope<'_, '_>) -> Option<()> { - let context = scope.get_current_context(); - let installed = context.get_slot::()?; - if !std::ptr::eq(self, installed.as_ref()) { - return None; - } + fn resolve_owner( + &self, + context: v8::Local<'_, v8::Context>, + ) -> Option { self.runtime()?; match &self.owner { - RuntimeOwner::Window { .. } => { - if page_context_is_disconnected(context) { + RuntimeOwner::Window { + host_ptr, + lifecycle, + } => { + if page_context_is_disconnected(context) + || lifecycle.upgrade()?.get() == ContextHostLifecycle::Destroyed + { return None; } - self.window_host_ptr()?; + Some(RuntimeExecutionContext::Window(*host_ptr)) } RuntimeOwner::Worker { state, .. } => { state.upgrade()?; + Some(RuntimeExecutionContext::Worker) } } - Some(()) } pub(crate) fn execution_context( &self, scope: &mut v8::PinScope<'_, '_>, ) -> Option { - self.validate_context(scope)?; - Some(match self.owner { - RuntimeOwner::Window { host_ptr, .. } => RuntimeExecutionContext::Window(host_ptr), - RuntimeOwner::Worker { .. } => RuntimeExecutionContext::Worker, - }) + let context = scope.get_current_context(); + let installed = context.get_slot::()?; + if !std::ptr::eq(self, installed.as_ref()) { + return None; + } + self.resolve_owner(context) + } + + /// Recheck the exact Context and owner immediately before author code. + /// Worker termination is checked in the same pass as owner resolution; + /// `close()` remains the task owner's separate delivery policy. + pub(crate) fn callback_execution_context( + &self, + scope: &mut v8::PinScope<'_, '_>, + ) -> Option { + let context = self.execution_context(scope)?; + if let RuntimeOwner::Worker { + termination_requested, + .. + } = &self.owner + && termination_requested.load(Ordering::Acquire) + { + return None; + } + Some(context) } pub(crate) fn window_host_ptr(&self) -> Option<*mut JsContextHost> { @@ -165,17 +194,6 @@ impl CommonRuntimeBinding { (lifecycle.upgrade()?.get() != ContextHostLifecycle::Destroyed).then_some(*host_ptr) } - pub(crate) fn worker_can_invoke(&self) -> bool { - self.runtime().is_some() - && match &self.owner { - RuntimeOwner::Worker { - state, - termination_requested, - } => state.upgrade().is_some() && !termination_requested.load(Ordering::Acquire), - RuntimeOwner::Window { .. } => false, - } - } - pub(crate) fn worker_can_deliver_task(&self) -> bool { self.runtime().is_some() && match &self.owner { @@ -187,6 +205,9 @@ impl CommonRuntimeBinding { } } +#[cfg(test)] +mod benchmarks; + #[cfg(test)] mod tests { use super::*; @@ -212,28 +233,34 @@ mod tests { CommonRuntimeBinding::install_window(other, &other_common, host, &other_lifecycle); let first_binding = { let scope = &mut v8::ContextScope::new(scope, first); - let binding = CommonRuntimeBinding::from_scope(scope).unwrap(); + let (binding, owner) = CommonRuntimeBinding::from_scope(scope).unwrap(); + assert!(matches!(owner, RuntimeExecutionContext::Window(ptr) if ptr == host)); assert!(Rc::ptr_eq(&binding.runtime().unwrap(), &common)); + assert!(binding.callback_execution_context(scope).is_some()); binding }; { let scope = &mut v8::ContextScope::new(scope, second); - let binding = CommonRuntimeBinding::from_scope(scope).unwrap(); + let (binding, _) = CommonRuntimeBinding::from_scope(scope).unwrap(); assert!(Rc::ptr_eq(&binding.runtime().unwrap(), &common)); assert!(!Rc::ptr_eq(&binding, &first_binding)); - assert!(first_binding.validate_context(scope).is_none()); + assert!(first_binding.execution_context(scope).is_none()); + assert!(first_binding.callback_execution_context(scope).is_none()); } { let scope = &mut v8::ContextScope::new(scope, other); - let binding = CommonRuntimeBinding::from_scope(scope).unwrap(); + let (binding, _) = CommonRuntimeBinding::from_scope(scope).unwrap(); assert!(!Rc::ptr_eq(&binding.runtime().unwrap(), &common)); other_lifecycle.set(ContextHostLifecycle::Destroyed); assert!(CommonRuntimeBinding::from_scope(scope).is_none()); + // A previously resolved owner is not a cached execution permit. + assert!(binding.callback_execution_context(scope).is_none()); } drop(common); let scope = &mut v8::ContextScope::new(scope, first); assert!(CommonRuntimeBinding::from_scope(scope).is_none()); assert!(first_binding.runtime().is_none()); + assert!(first_binding.callback_execution_context(scope).is_none()); } #[test] diff --git a/moli-renderer-v8/src/common_runtime/benchmarks.rs b/moli-renderer-v8/src/common_runtime/benchmarks.rs new file mode 100644 index 0000000000..e829adba73 --- /dev/null +++ b/moli-renderer-v8/src/common_runtime/benchmarks.rs @@ -0,0 +1,156 @@ +//! Manual binding microbenchmark. It measures native lookup/validation only, +//! with no JavaScript invocation or Window identity lookup. Run on an idle +//! machine with an optimized test binary: +//! +//! CARGO_PROFILE_RELEASE_CODEGEN_UNITS=256 cargo test --release -p moli-renderer-v8 +//! benchmark_common_runtime_binding_resolution -- --ignored --nocapture --test-threads=1 + +use std::{hint::black_box, time::Instant}; + +use super::*; + +// Keep the former call chain here as a benchmark reference, outside production +// code. Both paths use the same Context, owner, compiler, and sampling loop. +fn legacy_validate_context( + binding: &CommonRuntimeBinding, + scope: &mut v8::PinScope<'_, '_>, +) -> Option<()> { + let context = scope.get_current_context(); + let installed = context.get_slot::()?; + if !std::ptr::eq(binding, installed.as_ref()) { + return None; + } + binding.runtime()?; + match &binding.owner { + RuntimeOwner::Window { .. } => { + if page_context_is_disconnected(context) { + return None; + } + binding.window_host_ptr()?; + } + RuntimeOwner::Worker { state, .. } => { + state.upgrade()?; + } + } + Some(()) +} + +fn legacy_from_scope(scope: &mut v8::PinScope<'_, '_>) -> Option> { + let binding = scope + .get_current_context() + .get_slot::()?; + legacy_validate_context(&binding, scope)?; + Some(binding) +} + +fn legacy_execution_context( + binding: &CommonRuntimeBinding, + scope: &mut v8::PinScope<'_, '_>, +) -> Option { + legacy_validate_context(binding, scope)?; + Some(match binding.owner { + RuntimeOwner::Window { host_ptr, .. } => RuntimeExecutionContext::Window(host_ptr), + RuntimeOwner::Worker { .. } => RuntimeExecutionContext::Worker, + }) +} + +fn window_host(owner: RuntimeExecutionContext) -> Option<*mut JsContextHost> { + match owner { + RuntimeExecutionContext::Window(host) => Some(host), + RuntimeExecutionContext::Worker => None, + } +} + +fn sample( + scope: &mut v8::PinScope<'_, '_>, + iterations: usize, + operation: &mut impl FnMut(&mut v8::PinScope<'_, '_>) -> Option<*mut JsContextHost>, +) -> f64 { + // Bound V8's temporary handles to this sample, identically for both paths. + let scope = std::pin::pin!(v8::HandleScope::new(scope)); + let scope = &mut scope.init(); + let start = Instant::now(); + for _ in 0..iterations { + black_box(operation(black_box(&mut *scope))); + } + start.elapsed().as_secs_f64() * 1e9 / iterations as f64 +} + +fn compare( + scope: &mut v8::PinScope<'_, '_>, + label: &str, + mut legacy: impl FnMut(&mut v8::PinScope<'_, '_>) -> Option<*mut JsContextHost>, + mut resolved: impl FnMut(&mut v8::PinScope<'_, '_>) -> Option<*mut JsContextHost>, +) { + const ITERATIONS: usize = 500_000; + const SAMPLES: usize = 9; + assert!(legacy(scope).is_some()); + assert!(resolved(scope).is_some()); + sample(scope, 20_000, &mut legacy); + sample(scope, 20_000, &mut resolved); + let mut before = [0.0; SAMPLES]; + let mut after = [0.0; SAMPLES]; + for index in 0..SAMPLES { + // Alternate order to avoid consistently giving one path a warmer CPU. + if index % 2 == 0 { + before[index] = sample(scope, ITERATIONS, &mut legacy); + after[index] = sample(scope, ITERATIONS, &mut resolved); + } else { + after[index] = sample(scope, ITERATIONS, &mut resolved); + before[index] = sample(scope, ITERATIONS, &mut legacy); + } + } + before.sort_by(f64::total_cmp); + after.sort_by(f64::total_cmp); + println!( + "{label}: legacy median {:.1} ns/op [{:.1}, {:.1}], resolved median {:.1} ns/op [{:.1}, {:.1}]; {SAMPLES} samples x {ITERATIONS} iterations", + before[SAMPLES / 2], + before[0], + before[SAMPLES - 1], + after[SAMPLES / 2], + after[0], + after[SAMPLES - 1], + ); +} + +#[test] +#[ignore = "manual optimized microbenchmark; no performance thresholds in CI"] +fn benchmark_common_runtime_binding_resolution() { + crate::ensure_v8_for_test(); + let mut isolate = v8::Isolate::new(Default::default()); + let scope = std::pin::pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + let context = v8::Context::new(scope, Default::default()); + let common = Rc::new(CommonRuntimeState::default()); + let lifecycle = Rc::new(Cell::new(ContextHostLifecycle::Active)); + // Lookup does not dereference the host. Identity validation and JS + // invocation are deliberately outside this microbenchmark. + let host = std::ptr::NonNull::::dangling().as_ptr(); + CommonRuntimeBinding::install_window(context, &common, host, &lifecycle); + let scope = &mut v8::ContextScope::new(scope, context); + let (binding, _) = CommonRuntimeBinding::from_scope(scope).unwrap(); + compare( + scope, + "Window typed callback preparation", + |scope| legacy_from_scope(scope)?.window_host_ptr(), + |scope| window_host(CommonRuntimeBinding::from_scope(scope)?.1), + ); + compare( + scope, + "Window traced callback preparation", + |scope| { + let binding = legacy_from_scope(scope)?; + window_host(legacy_execution_context(&binding, scope)?) + }, + |scope| window_host(CommonRuntimeBinding::from_scope(scope)?.1), + ); + compare( + scope, + "Window pre-invocation binding validation", + |scope| { + legacy_execution_context(&binding, scope)?; + binding.window_host_ptr() + }, + |scope| window_host(binding.callback_execution_context(scope)?), + ); +} diff --git a/moli-renderer-v8/src/context_bootstrap/performance_observer_runtime/registration.rs b/moli-renderer-v8/src/context_bootstrap/performance_observer_runtime/registration.rs index c7d5092dab..c937a7194e 100644 --- a/moli-renderer-v8/src/context_bootstrap/performance_observer_runtime/registration.rs +++ b/moli-renderer-v8/src/context_bootstrap/performance_observer_runtime/registration.rs @@ -36,8 +36,8 @@ pub(super) fn register_performance_observer_callback<'s>( v8::Local<'s, v8::Object>, v8::Local<'s, v8::Object>, )> { - let binding = CommonRuntimeBinding::from_scope(scope)?; - if let RuntimeExecutionContext::Window(host_ptr) = binding.execution_context(scope)? { + let (_, owner) = CommonRuntimeBinding::from_scope(scope)?; + if let RuntimeExecutionContext::Window(host_ptr) = owner { let (id, callback, relevant, incumbent) = crate::observer_runtime::register_callback(scope, host_ptr, observer, callback) .into_parts(); @@ -58,8 +58,8 @@ impl<'s> PerformanceObserverRegistration<'s> { ) -> Option { let id = get_private_value(scope, observer, PERFORMANCE_OBSERVER_CALLBACK_ID_SLOT)? .number_value(scope)?; - let binding = CommonRuntimeBinding::from_scope(scope)?; - let environment = match binding.execution_context(scope)? { + let (binding, owner) = CommonRuntimeBinding::from_scope(scope)?; + let environment = match owner { RuntimeExecutionContext::Window(host_ptr) => RegistrationEnvironment::Window { host_ptr, callback_id: ObserverCallbackId::from_number(id)?, @@ -204,10 +204,10 @@ pub(super) fn performance_observer_callback_is_current<'s>( pub(super) fn active_performance_observers<'s>( scope: &mut v8::PinScope<'s, '_>, ) -> Vec> { - let Some(binding) = CommonRuntimeBinding::from_scope(scope) else { + let Some((_, owner)) = CommonRuntimeBinding::from_scope(scope) else { return Vec::new(); }; - if let Some(RuntimeExecutionContext::Window(host_ptr)) = binding.execution_context(scope) { + if let RuntimeExecutionContext::Window(host_ptr) = owner { return crate::observer_runtime::active_performance_observer_callbacks(scope, host_ptr); } let Some(registry) = global_queue_array(scope, PERFORMANCE_OBSERVER_REGISTRY_SLOT) else { diff --git a/moli-renderer-v8/src/v8_finalizer.rs b/moli-renderer-v8/src/v8_finalizer.rs index 022cf7519f..701d46ae4f 100644 --- a/moli-renderer-v8/src/v8_finalizer.rs +++ b/moli-renderer-v8/src/v8_finalizer.rs @@ -95,7 +95,7 @@ pub(crate) fn track_context_owned_v8_finalizer( let cleanup: FinalizerCleanup = Box::new(cleanup); let common = crate::common_runtime::CommonRuntimeBinding::from_scope(scope) - .and_then(|binding| binding.runtime()) + .and_then(|(binding, _)| binding.runtime()) .expect("V8 finalizer object must belong to a page or worker context"); let entry = ContextOwnedV8Finalizer::new(scope, object, cleanup); let retired = common.finalizers.borrow_mut().push(entry); diff --git a/moli-renderer-v8/src/webidl_callback_execution.rs b/moli-renderer-v8/src/webidl_callback_execution.rs index 65a21b72b4..73780f9fd0 100644 --- a/moli-renderer-v8/src/webidl_callback_execution.rs +++ b/moli-renderer-v8/src/webidl_callback_execution.rs @@ -50,8 +50,8 @@ impl PreparedWebIdlCallbackExecution { owner_identity: Option, callback_identity: WindowExecutionContextIdentity, ) -> Option { - let binding = CommonRuntimeBinding::from_scope(scope)?; - if binding.window_host_ptr()? != host_ptr { + let (binding, context) = CommonRuntimeBinding::from_scope(scope)?; + if !matches!(context, RuntimeExecutionContext::Window(installed) if installed == host_ptr) { return None; } Some(Self { @@ -69,11 +69,8 @@ impl PreparedWebIdlCallbackExecution { callback: PreparedWebIdlCallbackFunction, context: v8::Local<'s, v8::Context>, ) -> Option { - let binding = CommonRuntimeBinding::from_scope(scope)?; - if !matches!( - binding.execution_context(scope)?, - RuntimeExecutionContext::Worker - ) { + let (binding, owner) = CommonRuntimeBinding::from_scope(scope)?; + if !matches!(owner, RuntimeExecutionContext::Worker) { return None; } Some(Self { @@ -92,9 +89,9 @@ impl PreparedWebIdlCallbackExecution { scope: &mut v8::PinScope<'s, '_>, carrier: v8::Local<'s, v8::Object>, ) -> Option { - let binding = CommonRuntimeBinding::from_scope(scope)?; + let (binding, owner) = CommonRuntimeBinding::from_scope(scope)?; let callback = V8TracedWebIdlCallbackFunction::from_object(carrier).prepare(scope); - let environment = match binding.execution_context(scope)? { + let environment = match owner { RuntimeExecutionContext::Window(host_ptr) => { let host = unsafe { &*host_ptr }; let context = callback.relevant_context(scope); @@ -221,17 +218,14 @@ impl PreparedWebIdlCallbackExecution { impl CallbackExecutionEnvironment { fn can_invoke(&self, scope: &mut v8::PinScope<'_, '_>, binding: &CommonRuntimeBinding) -> bool { - if binding.execution_context(scope).is_none() { - return false; - } - match self { - Self::Window { - owner_identity, - callback_identity, - } => { - let Some(host_ptr) = binding.window_host_ptr() else { - return false; - }; + match (self, binding.callback_execution_context(scope)) { + ( + Self::Window { + owner_identity, + callback_identity, + }, + Some(RuntimeExecutionContext::Window(host_ptr)), + ) => { let host = unsafe { &*host_ptr }; let current = owner_identity.is_none_or(|identity| { host.window_execution_context_identity_is_current(identity) @@ -242,14 +236,14 @@ impl CallbackExecutionEnvironment { } current } - Self::Worker { context } => { + (Self::Worker { context }, Some(RuntimeExecutionContext::Worker)) => { scope.get_current_context() == v8::Local::new(scope, context) // `close()` stops future task delivery, but synchronous // APIs and already queued V8 microtasks may still run. // API/task owners enforce their own closing policy. - && binding.worker_can_invoke() && !scope.is_execution_terminating() } + _ => false, } }