Reviewers: metaweta,

Description:
This change reduces startup time of a minimal gadget
from ~1800ms to ~1100ms in Chrome Beta (16.0)

This change is mostly about speeding up cajaVM.def(), mainly
by changing a recursive implementation to an iterative one.

cajaVM.def also ends up invoking
  Object.prototype.ownKeys___
  whitelistAll
and this change speeds those up by using Object.keys when
possible, and inlining some tests.

Please review this at http://codereview.appspot.com/5493043/

Affected files:
  M     src/com/google/caja/es53.js
  M     src/com/google/caja/plugin/domado.js


Index: src/com/google/caja/es53.js
===================================================================
--- src/com/google/caja/es53.js (revision 4684)
+++ src/com/google/caja/es53.js (working copy)
@@ -222,9 +222,9 @@
   }

   var hasOwnProperty = Object.prototype.hasOwnProperty;
+  // In IE<9, this is an approximation of Object.keys, because a few own
+  // properties like 'toString' are never enumerable.
   var fastOwnKeys = Object.keys || function fastOwnKeys(o) {
-    // In IE<9, this will miss some keys, like an own 'toString', but that
- // doesn't matter, because we're only using this to find ___ properties.
     var keys = [];
     for (var k in o) {
       if (hasOwnProperty.call(o, k)) { keys.push(k); }
@@ -253,16 +253,17 @@
         this[P + '_gw___'] = false;
         this[P + '_w___'] = false;
       }
-      if (!this.NUM____v___ === this) { // inline this.hasNumerics___()
+      // inline this.hasNumerics___()
+      if (!this.NUM____v___ === this) {
         this.NUM____v___ = this;
         this.NUM____e___ = this;
         this.NUM____g___ = void 0;
         this.NUM____s___ = void 0;
       }
-      this['NUM____c___'] = false;
-      this['NUM____w___'] = false;
-      this['NUM____m___'] = false;
-      this['NUM____gw___'] = false;
+      this.NUM____c___ = false;
+      this.NUM____w___ = false;
+      this.NUM____m___ = false;
+      this.NUM____gw___ = false;
       // Make this non-extensible.
       this.ne___ = this;
       // Cache frozen state.
@@ -957,14 +958,15 @@

   Object.prototype.ownKeys___ = function () {
       var i, m, result = [];
-      for (i in this) {
-        if (!this.hasOwnProperty(i)) { continue; }
-        if (isNumericName(i)) {
+      var keys = fastOwnKeys(this);
+      for (var k = 0, n = keys.length; k < n; k++) {
+        var i = keys[k];
+        // inline isNumericName(i)
+        if (typeof i === 'number' || ('' + (+i)) === i) {
           result.push(i);
-        } else {
-          if (startsWithNUM___.test(i) && endsWith__.test(i)) { continue; }
-          m = i.match(endsWith_v___);
-          if (m) { result.push(m[1]); }
+        } else if (5 < i.length && i.substr(i.length - 5) === '_v___'
+                   && i.substr(0, 6) !== 'NUM___') {
+          result.push(i.substr(0, i.length - 5));
         }
       }
       return result;
@@ -1531,27 +1533,30 @@
   var origGetPrototypeOf, origGetOwnPropertyDescriptor;

   var def = markFuncFreeze(function(root) {
+    var i, n;
     var defending = newTable();
     var defendingList = [];
-    function recur(val) {
+    var todo = [root];
+    while (todo.length) {
+      var val = todo.pop();
       if (val !== Object(val) || defended.get(val) || defending.get(val)) {
-        return;
+        continue;
       }
       defending.set(val, true);
       defendingList.push(val);
       freeze(val);
-      recur(origGetPrototypeOf(val));
-      val.ownKeys___().forEach(function(p) {
-        var desc = origGetOwnPropertyDescriptor(val, p);
-        recur(desc.value);
-        recur(desc.get);
-        recur(desc.set);
-      });
+      todo.push(origGetPrototypeOf(val));
+      var keys = val.ownKeys___();
+      for (i = 0, n = keys.length; i < n; i++) {
+        var desc = origGetOwnPropertyDescriptor(val, keys[i]);
+        todo.push(desc.value);
+        todo.push(desc.get);
+        todo.push(desc.set);
+      }
     }
-    recur(root);
-    defendingList.forEach(function(obj) {
-      defended.set(obj, true);
-    });
+    for (i = 0, n = defendingList.length; i < n; i++) {
+      defended.set(defendingList[i], true);
+    }
     return root;
   });

@@ -3190,13 +3195,12 @@
    */
   function whitelistAll(obj, opt_deep) {
     var i;
-    for (i in obj) {
-      if (obj.hasOwnProperty(i) &&
-          !endsWith__.test(i) &&
-          !((i + '_v___') in obj)) {
-        var isObj = (typeof obj[i]) === 'object';
-        if (opt_deep && isObj) {
-          whitelistAll(obj[i], true);
+    var keys = fastOwnKeys(obj);
+    for (var k = 0, n = keys.length; k < n; k++) {
+      var i = keys[k], val = obj[i];
+      if (i.substr(i.length - 2) !== '__' && !((i + '_v___') in obj)) {
+        if (opt_deep && val && typeof val === 'object') {
+          whitelistAll(val, true);
         }
         obj[i + '_v___'] = obj;
         obj[i + '_w___'] = false;
@@ -3206,9 +3210,10 @@
         obj[i + '_g___'] = void 0;
         obj[i + '_s___'] = void 0;
         obj[i + '_m___'] = false;
-        if (isFunction(obj[i])) {
-          if (obj[i].f___ === Function.prototype.f___) {
-            markFunc(obj[i]);
+        if (val && val.f___ === Function.prototype.f___) {
+          // inline isFunction(val)
+          if (classProp.call(val) === '[object Function]') {
+            markFunc(val);
           }
         }
       }
Index: src/com/google/caja/plugin/domado.js
===================================================================
--- src/com/google/caja/plugin/domado.js        (revision 4684)
+++ src/com/google/caja/plugin/domado.js        (working copy)
@@ -327,7 +327,7 @@
      * This plays the role ___.grantTypedMethod did in Domita.
      */
     this.protectMethod = function (method) {
-      cajaVM.def(method);  // unnecessary in theory
+ //cajaVM.def(method); // unnecessary in theory. TODO(felix8a): verify
       function protectedMethod(var_args) {
         return method.apply(guard.coerce(this), arguments);
       }


Reply via email to