From d46db6d7f1f10d571e19b8fff9523f4de428947a Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 10 Apr 2015 12:56:46 +0200 Subject: [PATCH 1/4] Resume tracing our collections of roots. The second JS_SetExtraGCRootsTracer call clobbered the first, so trace_collections was no longer being called. --- components/script/dom/bindings/trace.rs | 2 +- components/script/script_task.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index f77b42648d9..4b5dffc1875 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -428,7 +428,7 @@ impl DerefMut for RootedVec { /// SM Callback that traces the rooted collections -pub unsafe extern fn trace_collections(tracer: *mut JSTracer, _data: *mut libc::c_void) { +pub unsafe fn trace_collections(tracer: *mut JSTracer) { ROOTED_COLLECTIONS.with(|ref collections| { let collections = collections.borrow(); collections.trace(tracer); diff --git a/components/script/script_task.rs b/components/script/script_task.rs index dbb73117857..7ef0b274167 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -99,12 +99,14 @@ use time::Tm; thread_local!(pub static STACK_ROOTS: Cell> = Cell::new(None)); thread_local!(static SCRIPT_TASK_ROOT: RefCell> = RefCell::new(None)); -unsafe extern fn trace_script_tasks(tr: *mut JSTracer, _data: *mut libc::c_void) { +unsafe extern fn trace_rust_roots(tr: *mut JSTracer, _data: *mut libc::c_void) { SCRIPT_TASK_ROOT.with(|root| { if let Some(script_task) = *root.borrow() { (*script_task).trace(tr); } }); + + trace_collections(tr); } /// A document load that is in the process of fetching the requested resource. Contains @@ -463,7 +465,7 @@ impl ScriptTask { unsafe { - JS_SetExtraGCRootsTracer((*js_runtime).ptr, Some(trace_collections), ptr::null_mut()); + JS_SetExtraGCRootsTracer((*js_runtime).ptr, Some(trace_rust_roots), ptr::null_mut()); } // Unconstrain the runtime's threshold on nominal heap size, to avoid // triggering GC too often if operating continuously near an arbitrary @@ -483,7 +485,6 @@ impl ScriptTask { js_context.set_logging_error_reporter(); unsafe { JS_SetGCZeal((*js_context).ptr, 0, JS_DEFAULT_ZEAL_FREQ); - JS_SetExtraGCRootsTracer((*js_runtime).ptr, Some(trace_script_tasks), ptr::null_mut()); } // Needed for debug assertions about whether GC is running. From 24ef5baf66d3a904cb400507c6baa975f9f23971 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 10 Apr 2015 12:40:29 +0200 Subject: [PATCH 2/4] Create a Runtime struct to manage JSRuntime and JSContext. I intend to move this struct into rust-mozjs, but I kept it here for easier iteration for now. --- components/script/script_task.rs | 69 ++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 7ef0b274167..7398f375a31 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -239,6 +239,49 @@ impl Drop for StackRootTLS { } } + +/// A wrapper for the `JSRuntime` and `JSContext` structures in SpiderMonkey. +pub struct Runtime { + rt: js::rust::rt, + cx: Rc, +} + +impl Runtime { + /// Creates a new `JSRuntime` and `JSContext`. + pub fn new() -> Runtime { + let js_runtime = js::rust::rt(); + assert!({ + let ptr: *mut JSRuntime = (*js_runtime).ptr; + !ptr.is_null() + }); + + // Unconstrain the runtime's threshold on nominal heap size, to avoid + // triggering GC too often if operating continuously near an arbitrary + // finite threshold. This leaves the maximum-JS_malloc-bytes threshold + // still in effect to cause periodical, and we hope hygienic, + // last-ditch GCs from within the GC's allocator. + unsafe { + JS_SetGCParameter(js_runtime.ptr, JSGC_MAX_BYTES, u32::MAX); + } + + let js_context = js_runtime.cx(); + assert!({ + let ptr: *mut JSContext = (*js_context).ptr; + !ptr.is_null() + }); + js_context.set_default_options_and_version(); + js_context.set_logging_error_reporter(); + unsafe { + JS_SetGCZeal((*js_context).ptr, 0, JS_DEFAULT_ZEAL_FREQ); + } + + Runtime { + rt: js_runtime, + cx: js_context, + } + } +} + /// Information for an entire page. Pages are top-level browsing contexts and can contain multiple /// frames. /// @@ -457,35 +500,11 @@ impl ScriptTask { pub fn new_rt_and_cx() -> (js::rust::rt, Rc) { LiveDOMReferences::initialize(); - let js_runtime = js::rust::rt(); - assert!({ - let ptr: *mut JSRuntime = (*js_runtime).ptr; - !ptr.is_null() - }); - + let Runtime { rt: js_runtime, cx: js_context } = Runtime::new(); unsafe { JS_SetExtraGCRootsTracer((*js_runtime).ptr, Some(trace_rust_roots), ptr::null_mut()); } - // Unconstrain the runtime's threshold on nominal heap size, to avoid - // triggering GC too often if operating continuously near an arbitrary - // finite threshold. This leaves the maximum-JS_malloc-bytes threshold - // still in effect to cause periodical, and we hope hygienic, - // last-ditch GCs from within the GC's allocator. - unsafe { - JS_SetGCParameter(js_runtime.ptr, JSGC_MAX_BYTES, u32::MAX); - } - - let js_context = js_runtime.cx(); - assert!({ - let ptr: *mut JSContext = (*js_context).ptr; - !ptr.is_null() - }); - js_context.set_default_options_and_version(); - js_context.set_logging_error_reporter(); - unsafe { - JS_SetGCZeal((*js_context).ptr, 0, JS_DEFAULT_ZEAL_FREQ); - } // Needed for debug assertions about whether GC is running. if !cfg!(ndebug) { From 2a5119ff372d306d1385f8061d817f7a3b1cdc7d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 10 Apr 2015 14:11:26 +0200 Subject: [PATCH 3/4] Add rt and cx methods to Runtime. --- components/script/script_task.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 7398f375a31..e1996bbdedf 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -280,6 +280,16 @@ impl Runtime { cx: js_context, } } + + /// Returns the `JSRuntime` object. + pub fn rt(&self) -> *mut JSRuntime { + self.rt.ptr + } + + /// Returns the `JSContext` object. + pub fn cx(&self) -> *mut JSContext { + self.cx.ptr + } } /// Information for an entire page. Pages are top-level browsing contexts and can contain multiple @@ -500,21 +510,21 @@ impl ScriptTask { pub fn new_rt_and_cx() -> (js::rust::rt, Rc) { LiveDOMReferences::initialize(); - let Runtime { rt: js_runtime, cx: js_context } = Runtime::new(); + let runtime = Runtime::new(); unsafe { - JS_SetExtraGCRootsTracer((*js_runtime).ptr, Some(trace_rust_roots), ptr::null_mut()); + JS_SetExtraGCRootsTracer(runtime.rt(), Some(trace_rust_roots), ptr::null_mut()); } // Needed for debug assertions about whether GC is running. if !cfg!(ndebug) { unsafe { - JS_SetGCCallback(js_runtime.ptr, + JS_SetGCCallback(runtime.rt(), Some(debug_gc_callback as unsafe extern "C" fn(*mut JSRuntime, JSGCStatus))); } } - (js_runtime, js_context) + (runtime.rt, runtime.cx) } // Return the root page in the frame tree. Panics if it doesn't exist. From 95e4e259245f67127aef673cc58b1112d5e416e2 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 10 Apr 2015 14:19:36 +0200 Subject: [PATCH 4/4] Move Runtime to rust-mozjs. --- components/script/script_task.rs | 58 ++------------------------------ components/servo/Cargo.lock | 2 +- ports/cef/Cargo.lock | 2 +- ports/gonk/Cargo.lock | 2 +- 4 files changed, 5 insertions(+), 59 deletions(-) diff --git a/components/script/script_task.rs b/components/script/script_task.rs index e1996bbdedf..6334dcb26a0 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -74,11 +74,10 @@ use util::task_state; use geom::Rect; use geom::point::Point2D; use hyper::header::{LastModified, Headers}; -use js::jsapi::{JS_SetWrapObjectCallbacks, JS_SetGCZeal, JS_SetExtraGCRootsTracer, JS_DEFAULT_ZEAL_FREQ}; +use js::jsapi::{JS_SetWrapObjectCallbacks, JS_SetExtraGCRootsTracer}; use js::jsapi::{JSContext, JSRuntime, JSObject, JSTracer}; -use js::jsapi::{JS_SetGCParameter, JSGC_MAX_BYTES}; use js::jsapi::{JS_SetGCCallback, JSGCStatus, JSGC_BEGIN, JSGC_END}; -use js::rust::{Cx, RtUtils}; +use js::rust::{Runtime, Cx, RtUtils}; use js; use url::Url; @@ -93,7 +92,6 @@ use std::ptr; use std::rc::Rc; use std::result::Result; use std::sync::mpsc::{channel, Sender, Receiver, Select}; -use std::u32; use time::Tm; thread_local!(pub static STACK_ROOTS: Cell> = Cell::new(None)); @@ -240,58 +238,6 @@ impl Drop for StackRootTLS { } -/// A wrapper for the `JSRuntime` and `JSContext` structures in SpiderMonkey. -pub struct Runtime { - rt: js::rust::rt, - cx: Rc, -} - -impl Runtime { - /// Creates a new `JSRuntime` and `JSContext`. - pub fn new() -> Runtime { - let js_runtime = js::rust::rt(); - assert!({ - let ptr: *mut JSRuntime = (*js_runtime).ptr; - !ptr.is_null() - }); - - // Unconstrain the runtime's threshold on nominal heap size, to avoid - // triggering GC too often if operating continuously near an arbitrary - // finite threshold. This leaves the maximum-JS_malloc-bytes threshold - // still in effect to cause periodical, and we hope hygienic, - // last-ditch GCs from within the GC's allocator. - unsafe { - JS_SetGCParameter(js_runtime.ptr, JSGC_MAX_BYTES, u32::MAX); - } - - let js_context = js_runtime.cx(); - assert!({ - let ptr: *mut JSContext = (*js_context).ptr; - !ptr.is_null() - }); - js_context.set_default_options_and_version(); - js_context.set_logging_error_reporter(); - unsafe { - JS_SetGCZeal((*js_context).ptr, 0, JS_DEFAULT_ZEAL_FREQ); - } - - Runtime { - rt: js_runtime, - cx: js_context, - } - } - - /// Returns the `JSRuntime` object. - pub fn rt(&self) -> *mut JSRuntime { - self.rt.ptr - } - - /// Returns the `JSContext` object. - pub fn cx(&self) -> *mut JSContext { - self.cx.ptr - } -} - /// Information for an entire page. Pages are top-level browsing contexts and can contain multiple /// frames. /// diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index d61b2571292..4d332f65ad2 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -479,7 +479,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.0" -source = "git+https://github.com/servo/rust-mozjs#9512c3c770774ed73a2fdcc635eee178cbd02ab1" +source = "git+https://github.com/servo/rust-mozjs#402b7b2db8816ffeccacfa9a8d316f4487e96ba0" dependencies = [ "libc 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "mozjs_sys 0.0.0 (git+https://github.com/servo/mozjs)", diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index 3fcab77d28b..5d9eedbe5ef 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -482,7 +482,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.0" -source = "git+https://github.com/servo/rust-mozjs#9512c3c770774ed73a2fdcc635eee178cbd02ab1" +source = "git+https://github.com/servo/rust-mozjs#402b7b2db8816ffeccacfa9a8d316f4487e96ba0" dependencies = [ "libc 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "mozjs_sys 0.0.0 (git+https://github.com/servo/mozjs)", diff --git a/ports/gonk/Cargo.lock b/ports/gonk/Cargo.lock index 7de597c61f9..07ef23c5293 100644 --- a/ports/gonk/Cargo.lock +++ b/ports/gonk/Cargo.lock @@ -415,7 +415,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.0" -source = "git+https://github.com/servo/rust-mozjs#9512c3c770774ed73a2fdcc635eee178cbd02ab1" +source = "git+https://github.com/servo/rust-mozjs#402b7b2db8816ffeccacfa9a8d316f4487e96ba0" dependencies = [ "libc 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "mozjs_sys 0.0.0 (git+https://github.com/servo/mozjs)",