Bug 1382645: Part 3 - Throw away description strings before blobbifying schema JSON. r=mixedpuppy
authorKris Maglione <maglione.k@gmail.com>
Tue, 25 Jul 2017 15:00:01 -0700
changeset 419699 f39eaceb3ca0d81717297e52e493eb2fca7df2ea
parent 419698 f6dc4970d2803673baab24a23f3dea9a08fbd342
child 419700 edded26e7141609581dfabd39298af699ab3abd6
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1382645
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1382645: Part 3 - Throw away description strings before blobbifying schema JSON. r=mixedpuppy MozReview-Commit-ID: 8rWQQhaTRr8
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -62,19 +62,55 @@ function readJSON(url) {
         resolve(JSON.parse(text));
       } catch (e) {
         reject(e);
       }
     });
   });
 }
 
+function stripDescriptions(json, stripThis = true) {
+  if (Array.isArray(json)) {
+    for (let i = 0; i < json.length; i++) {
+      if (typeof json[i] === "object" && json[i] !== null) {
+        json[i] = stripDescriptions(json[i]);
+      }
+    }
+    return json;
+  }
+
+  let result = {};
+
+  // Objects are handled much more efficiently, both in terms of memory and
+  // CPU, if they have the same shape as other objects that serve the same
+  // purpose. So, normalize the order of properties to increase the chances
+  // that the majority of schema objects wind up in large shape groups.
+  for (let key of Object.keys(json).sort()) {
+    if (stripThis && key === "description" && typeof json[key] === "string") {
+      continue;
+    }
+
+    if (typeof json[key] === "object" && json[key] !== null) {
+      result[key] = stripDescriptions(json[key], key !== "properties");
+    } else {
+      result[key] = json[key];
+    }
+  }
+
+  return result;
+}
+
 async function readJSONAndBlobbify(url) {
   let json = await readJSON(url);
 
+  // We don't actually use descriptions at runtime, and they make up about a
+  // third of the size of our structured clone data, so strip them before
+  // blobbifying.
+  json = stripDescriptions(json);
+
   return new StructuredCloneHolder(json);
 }
 
 /**
  * Defines a lazy getter for the given property on the given object. Any
  * security wrappers are waived on the object before the property is
  * defined, and the getter and setter methods are wrapped for the target
  * scope.
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas.js
@@ -620,40 +620,40 @@ add_task(async function() {
   tallied = null;
 
   Assert.throws(() => root.testing.pattern("DEADcow"),
                 /String "DEADcow" must match \/\^\[0-9a-f\]\+\$\/i/,
                 "should throw for non-match");
 
   root.testing.format({hostname: "foo"});
   verify("call", "testing", "format", [{hostname: "foo",
-                                        url: null,
                                         relativeUrl: null,
-                                        strictRelativeUrl: null}]);
+                                        strictRelativeUrl: null,
+                                        url: null}]);
   tallied = null;
 
   for (let invalid of ["", " ", "http://foo", "foo/bar", "foo.com/", "foo?"]) {
     Assert.throws(() => root.testing.format({hostname: invalid}),
                   /Invalid hostname/,
                   "should throw for invalid hostname");
   }
 
   root.testing.format({url: "http://foo/bar",
                        relativeUrl: "http://foo/bar"});
   verify("call", "testing", "format", [{hostname: null,
-                                        url: "http://foo/bar",
                                         relativeUrl: "http://foo/bar",
-                                        strictRelativeUrl: null}]);
+                                        strictRelativeUrl: null,
+                                        url: "http://foo/bar"}]);
   tallied = null;
 
   root.testing.format({relativeUrl: "foo.html", strictRelativeUrl: "foo.html"});
   verify("call", "testing", "format", [{hostname: null,
-                                        url: null,
                                         relativeUrl: `${wrapper.url}foo.html`,
-                                        strictRelativeUrl: `${wrapper.url}foo.html`}]);
+                                        strictRelativeUrl: `${wrapper.url}foo.html`,
+                                        url: null}]);
   tallied = null;
 
   for (let format of ["url", "relativeUrl"]) {
     Assert.throws(() => root.testing.format({[format]: "chrome://foo/content/"}),
                   /Access denied/,
                   "should throw for access denied");
   }
 
@@ -696,40 +696,40 @@ add_task(async function() {
   ];
   badDates.forEach(str => {
     Assert.throws(() => root.testing.formatDate({date: str}),
                   /Invalid date string/,
                   "should throw for invalid iso date string");
   });
 
   root.testing.deep({foo: {bar: [{baz: {required: 12, optional: "42"}}]}});
-  verify("call", "testing", "deep", [{foo: {bar: [{baz: {required: 12, optional: "42"}}]}}]);
+  verify("call", "testing", "deep", [{foo: {bar: [{baz: {optional: "42", required: 12}}]}}]);
   tallied = null;
 
   Assert.throws(() => root.testing.deep({foo: {bar: [{baz: {optional: "42"}}]}}),
                 /Type error for parameter arg \(Error processing foo\.bar\.0\.baz: Property "required" is required\) for testing\.deep/,
                 "should throw with the correct object path");
 
-  Assert.throws(() => root.testing.deep({foo: {bar: [{baz: {required: 12, optional: 42}}]}}),
+  Assert.throws(() => root.testing.deep({foo: {bar: [{baz: {optional: 42, required: 12}}]}}),
                 /Type error for parameter arg \(Error processing foo\.bar\.0\.baz\.optional: Expected string instead of 42\) for testing\.deep/,
                 "should throw with the correct object path");
 
 
   talliedErrors.length = 0;
 
-  root.testing.errors({warn: "0123", ignore: "0123", default: "0123"});
-  verify("call", "testing", "errors", [{warn: "0123", ignore: "0123", default: "0123"}]);
+  root.testing.errors({default: "0123", ignore: "0123", warn: "0123"});
+  verify("call", "testing", "errors", [{default: "0123", ignore: "0123", warn: "0123"}]);
   checkErrors([]);
 
-  root.testing.errors({warn: "0123", ignore: "x123", default: "0123"});
-  verify("call", "testing", "errors", [{warn: "0123", ignore: null, default: "0123"}]);
+  root.testing.errors({default: "0123", ignore: "x123", warn: "0123"});
+  verify("call", "testing", "errors", [{default: "0123", ignore: null,  warn: "0123"}]);
   checkErrors([]);
 
-  root.testing.errors({warn: "x123", ignore: "0123", default: "0123"});
-  verify("call", "testing", "errors", [{warn: null, ignore: "0123", default: "0123"}]);
+  root.testing.errors({default: "0123", ignore: "0123", warn: "x123"});
+  verify("call", "testing", "errors", [{default: "0123", ignore: "0123", warn: null}]);
   checkErrors([
     'String "x123" must match /^\\d+$/',
   ]);
 
 
   root.testing.onFoo.addListener(f);
   do_check_eq(JSON.stringify(tallied.slice(0, -1)), JSON.stringify(["addListener", "testing", "onFoo"]));
   do_check_eq(tallied[3][0], f);
@@ -778,17 +778,17 @@ add_task(async function() {
     let stringProxy = new Proxy(stringTarget, {});
     Assert.throws(() => root.testing.quack(stringProxy),
                   /Expected a plain JavaScript object, got a Proxy/,
                   "should throw when passing a Proxy");
   }
 
 
   root.testing.localize({foo: "__MSG_foo__", bar: "__MSG_foo__", url: "__MSG_http://example.com/__"});
-  verify("call", "testing", "localize", [{foo: "FOO", bar: "__MSG_foo__", url: "http://example.com/"}]);
+  verify("call", "testing", "localize", [{bar: "__MSG_foo__", foo: "FOO", url: "http://example.com/"}]);
   tallied = null;
 
 
   Assert.throws(() => root.testing.localize({url: "__MSG_/foo/bar__"}),
                 /\/FOO\/BAR is not a valid URL\./,
                 "should throw for invalid URL");