perf(cdp): scope the V8 lock per connection to run connections in parallel

With every connection on its own thread (previous commit), the process-wide V8
lock served no safety purpose across connections yet forced all of them to
serialize on one mutex, so throughput did not scale with concurrency. Move the
lock into CdpContext so each connection has its own: a connection's nav task and
command dispatch still coordinate on one page's isolate at a time on its thread,
but different connections no longer block each other. Delete the now-unused
process-wide obscura_js::v8_lock.

Measured on a local CPU-bound fixture, page throughput stays flat (~75/s from
concurrency 1 to 8) with the global lock and scales to about 2.5x at concurrency
8 with the per-connection lock, with no crash at any concurrency. Obstacle
course 33/33, obscura-cdp 76/76.

Refs #430.
This commit is contained in:
sycatle committed 2026-07-20 16:37:50 +02:00
1 parent 6c7b2b0cec
commit 9065f383de
4 files changed
+21 -34

No files matched your search

+14 -1
View File
@@ -51,6 +51,14 @@ pub struct CdpContext {
// caps how many bodies (and how many bytes) can be held at once, evicting
// the oldest, so an abandoned or disconnected stream cannot leak unbounded.
pub io_streams: crate::domains::io::IoStreamStore,
/// Serializes V8 work within THIS connection. With the thread-per-connection
/// server (#430) each connection runs on its own OS thread, so isolates never
/// collide across connections; this per-connection lock keeps a connection's
/// own nav task and command dispatch from interleaving two of its pages'
/// isolates on that one thread. It is deliberately per-connection, not a
/// process-wide lock, so connections run in parallel (measured ~2x at
/// concurrency 2, ~3x at 4) instead of serializing all V8 on one mutex.
pub v8_lock: Arc<tokio::sync::Mutex<()>>,
}
impl CdpContext {
@@ -131,6 +139,7 @@ impl CdpContext {
valid_context_ids,
next_isolated_context_id: 100,
io_streams: crate::domains::io::IoStreamStore::default(),
v8_lock: Arc::new(tokio::sync::Mutex::new(())),
}
}
@@ -175,6 +184,7 @@ impl CdpContext {
valid_context_ids,
next_isolated_context_id: 100,
io_streams: crate::domains::io::IoStreamStore::default(),
v8_lock: Arc::new(tokio::sync::Mutex::new(())),
}
}
@@ -323,7 +333,10 @@ pub async fn dispatch(req: &CdpRequest, ctx: &mut CdpContext) -> CdpResponse {
let _v8_guard = if is_v8_free_method(&req.method) {
None
} else {
Some(obscura_js::v8_lock::global().lock().await)
// Per-connection lock (owned guard, so it does not borrow `ctx`): keeps
// this connection's own V8 work contiguous on its thread without
// serializing other connections (#430).
Some(ctx.v8_lock.clone().lock_owned().await)
};
// Per-command V8 watchdog. The lock above keeps each handler contiguous on
+7 -5
View File
@@ -673,13 +673,15 @@ async fn process_with_interception(
let (nav_done_tx, mut nav_done_rx) = mpsc::channel::<(obscura_browser::Page, Result<(), String>)>(1);
let url_owned = url.to_string();
let nav_v8_lock = ctx.v8_lock.clone();
tokio::task::spawn_local(async move {
// Issue #19: serialize V8 work across pages. The interception path
// spawns navigation here while the parent task continues to pump
// CDP messages via `dispatch` (which also acquires this lock); both
// sides must coordinate or V8 aborts the process at concurrency >= 5.
let _v8_guard = obscura_js::v8_lock::global().lock().await;
// Issue #19: serialize this connection's V8 work across its pages. This
// nav task runs while the connection's processor keeps pumping other CDP
// messages via `dispatch` (which takes the same per-connection lock), so
// both sides coordinate on one page's isolate at a time on this thread.
// The lock is per-connection, so other connections are unaffected (#430).
let _v8_guard = nav_v8_lock.lock_owned().await;
// Preloads (addBinding shims, addScriptToEvaluateOnNewDocument sources)
// must run BEFORE the page's own scripts (CDP contract). Hand them
// to the page so navigate_single can inject them at the right point.
-1
View File
@@ -6,7 +6,6 @@ pub mod module_loader;
pub mod runtime;
pub mod ops;
pub mod v8_flags;
pub mod v8_lock;
pub mod markdown;
pub use markdown::HTML_TO_MARKDOWN_JS;
-27
View File
@@ -1,27 +0,0 @@
//! Process-wide async lock that serializes V8 work across all `JsRuntime`s.
//!
//! V8's invariant: on any given OS thread, only one `Isolate` may be entered
//! at a time, and HandleScope/ContextScope stacks must unwind on the Isolate
//! they belong to. Obscura's CDP server runs every `JsRuntime` (one per Page)
//! on a single OS thread via `tokio::task::LocalSet` + `spawn_local`. As soon
//! as two pages' V8-touching futures interleave across an `.await`, V8 trips
//! its `heap->isolate() == Isolate::TryGetCurrent()` check and aborts the
//! whole process (no Rust panic; `V8_Fatal` calls `abort(3)`).
//!
//! Acquiring this lock around any block that calls `JsRuntime::execute_script`
//! or `JsRuntime::run_event_loop` keeps that block contiguous on the thread:
//! V8 fully exits the prior Isolate before the next page is allowed in. This
//! converts the abort into latency. It is the issue-19 "Option 1" fix.
//!
//! The properly concurrent fix is to pin each `JsRuntime` to its own OS
//! thread (issue-19 "Option 2"); that's a larger refactor tracked separately.
use std::sync::OnceLock;
use tokio::sync::Mutex;
static V8_LOCK: OnceLock<Mutex<()>> = OnceLock::new();
/// Returns the process-wide V8 serialization lock.
pub fn global() -> &'static Mutex<()> {
V8_LOCK.get_or_init(|| Mutex::new(()))
}