From b49acf40baf2e8559c5a067c609323c2b5400eef Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 16 Mar 2018 16:50:05 +0100 Subject: [PATCH 1/5] Use mozjs 0.3 fork without Heap::new --- Cargo.lock | 10 +++++----- Cargo.toml | 2 +- components/malloc_size_of/Cargo.toml | 2 +- components/script/Cargo.toml | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 63096eaceb0..b0b28068b66 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1617,7 +1617,7 @@ dependencies = [ "cssparser 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.17.2 (registry+https://github.com/rust-lang/crates.io-index)", "hashglobe 0.1.0", - "mozjs 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "mozjs 0.3.0 (git+https://github.com/Xanewok/rust-mozjs?branch=remove-heap-constructor)", "selectors 0.19.0", "servo_arc 0.1.1", "smallbitvec 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1792,8 +1792,8 @@ dependencies = [ [[package]] name = "mozjs" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" +version = "0.3.0" +source = "git+https://github.com/Xanewok/rust-mozjs?branch=remove-heap-constructor#d8b6ae7fcff342c2fddca8df5a7556bcc3b9e415" dependencies = [ "cmake 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2459,7 +2459,7 @@ dependencies = [ "mime_guess 1.8.1 (registry+https://github.com/rust-lang/crates.io-index)", "mitochondria 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "mozangle 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", - "mozjs 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "mozjs 0.3.0 (git+https://github.com/Xanewok/rust-mozjs?branch=remove-heap-constructor)", "msg 0.0.1", "net_traits 0.0.1", "num-traits 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3842,7 +3842,7 @@ dependencies = [ "checksum miow 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "8c1f2f3b1cf331de6896aabf6e9d55dca90356cc9960cca7eaaf408a355ae919" "checksum mitochondria 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "9de3eca27871df31c33b807f834b94ef7d000956f57aa25c5aed9c5f0aae8f6f" "checksum mozangle 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "1f0583e6792917f498bb3a7440f777a59353102063445ab7f5e9d1dc4ed593aa" -"checksum mozjs 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "79645a302ffcef7ca2673ff4db4d4273b87671dd0de4d53da889b664dd555e74" +"checksum mozjs 0.3.0 (git+https://github.com/Xanewok/rust-mozjs?branch=remove-heap-constructor)" = "" "checksum mozjs_sys 0.50.1 (registry+https://github.com/rust-lang/crates.io-index)" = "e61a792a125b1364c5ec50255ed8343ce02dc56098f8868dd209d472c8de006a" "checksum mp3-metadata 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4ab5f1d2693586420208d1200ce5a51cd44726f055b635176188137aff42c7de" "checksum mp4parse 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f821e3799bc0fd16d9b861fb02fa7ee1b5fba29f45ad591dade105c48ca9a1a0" diff --git a/Cargo.toml b/Cargo.toml index d42024cc101..233d40e5b7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ opt-level = 3 # lto = false [patch.crates-io] - +mozjs = { git = "https://github.com/Xanewok/rust-mozjs", branch = "remove-heap-constructor" } # If you need to temporarily test Servo with a local fork of some upstream # crate, add that here. Use the form: # diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index ced9b0eb998..b9cb45d4126 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -16,7 +16,7 @@ app_units = "0.6" cssparser = "0.23.0" euclid = "0.17" hashglobe = { path = "../hashglobe" } -mozjs = { version = "0.2", features = ["promises"], optional = true } +mozjs = { version = "0.3", features = ["promises"], optional = true } selectors = { path = "../selectors" } servo_arc = { path = "../servo_arc" } smallbitvec = "1.0.3" diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index cf16a2f8a80..0d2b8a5b3d0 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -62,7 +62,7 @@ metrics = {path = "../metrics"} mitochondria = "1.1.2" mime = "0.2.1" mime_guess = "1.8.0" -mozjs = { version = "0.2", features = ["promises"]} +mozjs = { version = "0.3", features = ["promises"]} msg = {path = "../msg"} net_traits = {path = "../net_traits"} num-traits = "0.1.32" From 760e0a5b5792fb3c8967aee53cc41e1e25adf07d Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 13 Mar 2018 14:57:09 +0100 Subject: [PATCH 2/5] Root JS object members in dictionaries --- components/script/dom/bindings/codegen/CodegenRust.py | 8 +++----- components/script/dom/testbinding.rs | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 1207b050797..916c8770d34 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1121,13 +1121,10 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, templateBody = "${val}.get().to_object()" default = "ptr::null_mut()" - # TODO: Do we need to do the same for dictionaries? - if isMember == "Union": + if isMember in ("Dictionary", "Union"): templateBody = "RootedTraceableBox::from_box(Heap::boxed(%s))" % templateBody default = "RootedTraceableBox::new(Heap::default())" declType = CGGeneric("RootedTraceableBox>") - elif isMember == "Dictionary": - declType = CGGeneric("Heap<*mut JSObject>") else: # TODO: Need to root somehow # https://github.com/servo/servo/issues/6382 @@ -6168,7 +6165,8 @@ class CGDictionary(CGThing): conversion = self.getMemberConversion(memberInfo, member.type) if isInitial: return CGGeneric("%s: %s,\n" % (name, conversion.define())) - if member.type.isAny() or member.type.isObject(): + # TODO: Root Heap using RootedTraceableBox + if member.type.isAny(): return CGGeneric("dictionary.%s.set(%s);\n" % (name, conversion.define())) return CGGeneric("dictionary.%s = %s;\n" % (name, conversion.define())) diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 80159a30068..d450c282885 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -369,7 +369,7 @@ impl TestBindingMethods for TestBinding { nullableFloatValue: None, nullableLongLongValue: None, nullableLongValue: None, - nullableObjectValue: Heap::default(), + nullableObjectValue: RootedTraceableBox::new(Heap::default()), nullableOctetValue: None, nullableShortValue: None, nullableStringValue: None, From 64dc0c4b9eeba6f5c56a917a0d32125df9248ed2 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 16 Mar 2018 15:54:36 +0100 Subject: [PATCH 3/5] Root `any` members in dictionaries --- .../dom/bindings/codegen/CodegenRust.py | 23 ++++++++++--------- components/script/dom/bindings/conversions.rs | 21 ++++++++++++++++- components/script/dom/bindings/iterable.rs | 10 ++++---- components/script/dom/testbinding.rs | 4 ++-- 4 files changed, 39 insertions(+), 19 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 916c8770d34..24054de8e95 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1084,13 +1084,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, assert isMember != "Union" if isMember == "Dictionary" or isAutoRooted: - # TODO: Need to properly root dictionaries - # https://github.com/servo/servo/issues/6381 - if isMember == "Dictionary": - declType = CGGeneric("Heap") - # AutoRooter can trace properly inner raw GC thing pointers - else: - declType = CGGeneric("JSVal") + templateBody = "${val}.get()" if defaultValue is None: default = None @@ -1100,7 +1094,17 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, default = "UndefinedValue()" else: raise TypeError("Can't handle non-null, non-undefined default value here") - return handleOptional("${val}.get()", declType, default) + + if isMember == "Dictionary": + templateBody = "RootedTraceableBox::from_box(Heap::boxed(%s))" % templateBody + if default is not None: + default = "RootedTraceableBox::from_box(Heap::boxed(%s))" % default + declType = CGGeneric("RootedTraceableBox>") + # AutoRooter can trace properly inner raw GC thing pointers + else: + declType = CGGeneric("JSVal") + + return handleOptional(templateBody, declType, default) declType = CGGeneric("HandleValue") @@ -6165,9 +6169,6 @@ class CGDictionary(CGThing): conversion = self.getMemberConversion(memberInfo, member.type) if isInitial: return CGGeneric("%s: %s,\n" % (name, conversion.define())) - # TODO: Root Heap using RootedTraceableBox - if member.type.isAny(): - return CGGeneric("dictionary.%s.set(%s);\n" % (name, conversion.define())) return CGGeneric("dictionary.%s = %s;\n" % (name, conversion.define())) def varInsert(varName, dictionaryName): diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index e40a1d7092b..3824ed33183 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -48,7 +48,7 @@ use js::error::throw_type_error; use js::glue::{GetProxyPrivate, IsWrapper}; use js::glue::{RUST_JSID_IS_INT, RUST_JSID_TO_INT}; use js::glue::{RUST_JSID_IS_STRING, RUST_JSID_TO_STRING, UnwrapObject}; -use js::jsapi::{HandleId, HandleObject, HandleValue, JSContext, JSObject, JSString}; +use js::jsapi::{HandleId, HandleObject, HandleValue, Heap, JSContext, JSObject, JSString}; use js::jsapi::{JS_GetLatin1StringCharsAndLength, JS_GetProperty, JS_GetReservedSlot}; use js::jsapi::{JS_GetTwoByteStringCharsAndLength, JS_IsArrayObject, JS_IsExceptionPending}; use js::jsapi::{JS_NewStringCopyN, JS_StringHasLatin1Chars, MutableHandleValue}; @@ -125,6 +125,25 @@ impl ToJSValConvertible for RootedTraceable } } +impl FromJSValConvertible for RootedTraceableBox> + where + T: FromJSValConvertible + js::rust::GCMethods + Copy, + Heap: JSTraceable + Default +{ + type Config = T::Config; + + unsafe fn from_jsval(cx: *mut JSContext, + value: HandleValue, + config: Self::Config) + -> Result, ()> { + T::from_jsval(cx, value, config).map(|result| match result { + ConversionResult::Success(inner) => + ConversionResult::Success(RootedTraceableBox::from_box(Heap::boxed(inner))), + ConversionResult::Failure(msg) => ConversionResult::Failure(msg), + }) + } +} + /// Convert `id` to a `DOMString`. Returns `None` if `id` is not a string or /// integer. /// diff --git a/components/script/dom/bindings/iterable.rs b/components/script/dom/bindings/iterable.rs index 15fbe7175cb..60331f6608e 100644 --- a/components/script/dom/bindings/iterable.rs +++ b/components/script/dom/bindings/iterable.rs @@ -11,7 +11,7 @@ use dom::bindings::codegen::Bindings::IterableIteratorBinding::IterableKeyOrValu use dom::bindings::error::Fallible; use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; use dom::bindings::root::{Dom, DomRoot}; -use dom::bindings::trace::JSTraceable; +use dom::bindings::trace::{JSTraceable, RootedTraceableBox}; use dom::globalscope::GlobalScope; use dom_struct::dom_struct; use js::conversions::ToJSValConvertible; @@ -131,10 +131,10 @@ fn key_and_value_return(cx: *mut JSContext, value: HandleValue) -> Fallible<()> { let mut dict = unsafe { IterableKeyAndValueResult::empty(cx) }; dict.done = false; - let values = vec![Heap::default(), Heap::default()]; - values[0].set(key.get()); - values[1].set(value.get()); - dict.value = Some(values); + dict.value = Some(vec![key, value] + .into_iter() + .map(|handle| RootedTraceableBox::from_box(Heap::boxed(handle.get()))) + .collect()); rooted!(in(cx) let mut dict_value = UndefinedValue()); unsafe { dict.to_jsval(cx, dict_value.handle_mut()); diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index d450c282885..a5bb29a7e76 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -348,12 +348,12 @@ impl TestBindingMethods for TestBinding { fn ReceiveNullableSequence(&self) -> Option> { Some(vec![1]) } fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> RootedTraceableBox { RootedTraceableBox::new(TestDictionary { - anyValue: Heap::default(), + anyValue: RootedTraceableBox::new(Heap::default()), booleanValue: None, byteValue: None, dict: RootedTraceableBox::new(TestDictionaryDefaults { UnrestrictedDoubleValue: 0.0, - anyValue: Heap::default(), + anyValue: RootedTraceableBox::new(Heap::default()), booleanValue: false, bytestringValue: ByteString::new(vec![]), byteValue: 0, From 30fb15df233343d93dc8cdf18f9709ac03d798bd Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sun, 18 Mar 2018 13:17:09 +0100 Subject: [PATCH 4/5] Add key/value iterable HTML benchmark --- tests/html/pair_iterable_perf.html | 44 ++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/html/pair_iterable_perf.html diff --git a/tests/html/pair_iterable_perf.html b/tests/html/pair_iterable_perf.html new file mode 100644 index 00000000000..aa064db2054 --- /dev/null +++ b/tests/html/pair_iterable_perf.html @@ -0,0 +1,44 @@ + + +Value and pair iterable bindings + From 0ea6d8ac3e4293cb46ee195fd8e0771c1b9ccc72 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sun, 18 Mar 2018 13:31:44 +0100 Subject: [PATCH 5/5] Fix tabs --- tests/html/pair_iterable_perf.html | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/html/pair_iterable_perf.html b/tests/html/pair_iterable_perf.html index aa064db2054..f4fefc6ba68 100644 --- a/tests/html/pair_iterable_perf.html +++ b/tests/html/pair_iterable_perf.html @@ -12,11 +12,11 @@ } function measure_time(func) { - var start = performance.now(); - func(); - var stop = performance.now(); + var start = performance.now(); + func(); + var stop = performance.now(); - return (stop - start) / 1000.0; + return (stop - start) / 1000.0; }; const ENTRY_COUNT = 10000; @@ -29,13 +29,13 @@ t.add(i.toString(), i); } - var result = collect(t.entries()); + var result = collect(t.entries()); }; var avg = 0; for (var i = 0; i < RUN_COUNT; i++) { var time = measure_time(benchMe); - avg += time; + avg += time; } avg /= RUN_COUNT;