Bug 1406311 - string-format: optimise string-format for %S patterns;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 12 Oct 2017 14:34:24 +0200
changeset 680934 19db44fe15d2deb3723b4ed0dd5e47b161cee8a9
parent 680933 93216fe3a4d098caf06c28c9df4a5543ba71bdad
child 680935 db2e708e719b04354afb1142f506abc3fde9b07d
push id84682
push userjdescottes@mozilla.com
push dateMon, 16 Oct 2017 16:12:33 +0000
reviewersbgrins
bugs1406311
milestone58.0a1
Bug 1406311 - string-format: optimise string-format for %S patterns;r=bgrins Most devtools usage of string formatting is with %S placeholders. We can speed it the formatting by checking for those patterns first. For very simple patterns using a single %S placeholder, we can even just use String replace. MozReview-Commit-ID: 2dOZBy7sHfS
devtools/shared/string-format.js
--- a/devtools/shared/string-format.js
+++ b/devtools/shared/string-format.js
@@ -43,29 +43,42 @@ const re = {
   // eslint-disable-next-line max-len
   placeholder: /^\x25(?:([1-9]\d*)\$|\(([^\)]+)\))?(\+)?(0|'[^$])?(-)?(\d+)?(?:\.(\d+))?([b-gijosStTuvxX])/,
   key: /^([a-z_][a-z_\d]*)/i,
   keyAccess: /^\.([a-z_][a-z_\d]*)/i,
   indexAccess: /^\[(\d+)\]/,
   sign: /^[\+\-]/
 };
 
+const SIMPLE_STRING_PATTERN = "SIMPLE_STRING_PATTERN";
+
 const cache = {};
 function stringFormat() {
   let key = arguments[0];
   if (!(cache[key] && cache.hasOwnProperty(key))) {
     cache[key] = stringFormat.parse(key);
   }
   return stringFormat.format.call(null, cache[key], arguments);
 }
 
 // eslint-disable-next-line complexity
 stringFormat.format = function (parseTree, argv) {
+  // For simple string patterns, simply use String.prototype.replace.
+  if (parseTree === SIMPLE_STRING_PATTERN) {
+    let arg = argv[1];
+    if (typeof arg == "function") {
+      arg = arg();
+    } else if (typeof arg != "string") {
+      arg = String(arg);
+    }
+    return argv[0].replace("%S", arg);
+  }
+
   let cursor = 1, treeLength = parseTree.length, nodeType = "", arg, output = [], i, k,
-    match, pad, padCharacter, padLength, isPositive = true, sign = "";
+    match;
   for (i = 0; i < treeLength; i++) {
     nodeType = typeof parseTree[i];
     // parseTree item is either a string or an array (return of match() call).
     if (nodeType === "string") {
       output[output.length] = parseTree[i];
     } else if (nodeType === "object") {
       match = parseTree[i]; // convenience purposes only
       if (match[2]) { // keyword argument
@@ -78,123 +91,148 @@ stringFormat.format = function (parseTre
           arg = arg[match[2][k]];
         }
       } else if (match[1]) { // positional argument (explicit)
         arg = argv[match[1]];
       } else { // positional argument (implicit)
         arg = argv[cursor++];
       }
 
-      if (re.notType.test(match[8]) && re.notPrimitive.test(match[8]) &&
-        typeof arg == "function") {
-        arg = arg();
-      }
-
-      if (re.numericArg.test(match[8]) && (typeof arg != "number" && isNaN(arg))) {
-        throw new TypeError(stringFormat("[stringFormat] expecting number but found %s",
-          typeof arg));
-      }
-
-      if (re.number.test(match[8])) {
-        isPositive = arg >= 0;
-      }
-
-      switch (match[8]) {
-        case "b":
-          arg = parseInt(arg, 10).toString(2);
-          break;
-        case "c":
-          arg = String.fromCharCode(parseInt(arg, 10));
-          break;
-        case "d":
-        case "i":
-          arg = parseInt(arg, 10);
-          break;
-        case "j":
-          arg = JSON.stringify(arg, null, match[6] ? parseInt(match[6], 10) : 0);
-          break;
-        case "e":
-          arg = match[7] ? parseFloat(arg).toExponential(match[7]) :
-            parseFloat(arg).toExponential();
-          break;
-        case "f":
-          arg = match[7] ? parseFloat(arg).toFixed(match[7]) : parseFloat(arg);
-          break;
-        case "g":
-          arg = match[7] ? parseFloat(arg).toPrecision(match[7]) : parseFloat(arg);
-          break;
-        case "o":
-          arg = arg.toString(8);
-          break;
-        case "s":
-        case "S":
+      // The most commonly used type in DevTools is the string type with no padding,
+      // process it in priority to avoid unnecessary checks.
+      let isStringType = match[8] === "S" || match[8] === "s";
+      let hasPadding = match[6];
+      if (isStringType && !hasPadding) {
+        if (typeof arg == "function") {
+          arg = arg();
+        } else if (typeof arg != "string") {
           arg = String(arg);
-          arg = (match[7] ? arg.substring(0, match[7]) : arg);
-          break;
-        case "t":
-          arg = String(!!arg);
-          arg = (match[7] ? arg.substring(0, match[7]) : arg);
-          break;
-        case "T":
-          arg = Object.prototype.toString.call(arg).slice(8, -1).toLowerCase();
-          arg = (match[7] ? arg.substring(0, match[7]) : arg);
-          break;
-        case "u":
-          arg = parseInt(arg, 10) >>> 0;
-          break;
-        case "v":
-          arg = arg.valueOf();
-          arg = (match[7] ? arg.substring(0, match[7]) : arg);
-          break;
-        case "x":
-          arg = parseInt(arg, 10).toString(16);
-          break;
-        case "X":
-          arg = parseInt(arg, 10).toString(16).toUpperCase();
-          break;
-      }
-      if (re.json.test(match[8])) {
-        output[output.length] = arg;
+        }
+        output[output.length] = (match[7] ? arg.substring(0, match[7]) : arg);
       } else {
-        if (re.number.test(match[8]) && (!isPositive || match[3])) {
-          sign = isPositive ? "+" : "-";
-          arg = arg.toString().replace(re.sign, "");
-        } else {
-          sign = "";
-        }
-
-        if (match[4]) {
-          padCharacter = match[4] === "0" ? "0" : match[4].charAt(1);
-        } else {
-          padCharacter = " ";
-        }
-
-        padLength = match[6] - (sign + arg).length;
-
-        if (match[6] && padLength > 0) {
-          pad = strRepeat(padCharacter, padLength);
-        } else {
-          pad = "";
-        }
-
-        if (match[5]) {
-          output[output.length] = sign + arg + pad;
-        } else if (padCharacter === "0") {
-          output[output.length] = sign + pad + arg;
-        } else {
-          output[output.length] = pad + sign + arg;
-        }
+        output[output.length] = processComplexType(arg, match);
       }
     }
   }
   return output.join("");
 };
 
+// eslint-disable-next-line complexity
+const processComplexType = function (arg, match) {
+  let pad, padCharacter, padLength, isPositive = true, sign = "";
+
+  if (typeof arg == "function" && re.notType.test(match[8]) &&
+    re.notPrimitive.test(match[8])) {
+    arg = arg();
+  }
+
+  if (re.numericArg.test(match[8]) && (typeof arg != "number" && isNaN(arg))) {
+    throw new TypeError(stringFormat("[stringFormat] expecting number but found %s",
+      typeof arg));
+  }
+
+  if (re.number.test(match[8])) {
+    isPositive = arg >= 0;
+  }
+
+  switch (match[8]) {
+    case "b":
+      arg = parseInt(arg, 10).toString(2);
+      break;
+    case "c":
+      arg = String.fromCharCode(parseInt(arg, 10));
+      break;
+    case "d":
+    case "i":
+      arg = parseInt(arg, 10);
+      break;
+    case "j":
+      arg = JSON.stringify(arg, null, match[6] ? parseInt(match[6], 10) : 0);
+      break;
+    case "e":
+      arg = match[7] ? parseFloat(arg).toExponential(match[7]) :
+        parseFloat(arg).toExponential();
+      break;
+    case "f":
+      arg = match[7] ? parseFloat(arg).toFixed(match[7]) : parseFloat(arg);
+      break;
+    case "g":
+      arg = match[7] ? parseFloat(arg).toPrecision(match[7]) : parseFloat(arg);
+      break;
+    case "o":
+      arg = arg.toString(8);
+      break;
+    case "s":
+    case "S":
+      arg = String(arg);
+      arg = (match[7] ? arg.substring(0, match[7]) : arg);
+      break;
+    case "t":
+      arg = String(!!arg);
+      arg = (match[7] ? arg.substring(0, match[7]) : arg);
+      break;
+    case "T":
+      arg = Object.prototype.toString.call(arg).slice(8, -1).toLowerCase();
+      arg = (match[7] ? arg.substring(0, match[7]) : arg);
+      break;
+    case "u":
+      arg = parseInt(arg, 10) >>> 0;
+      break;
+    case "v":
+      arg = arg.valueOf();
+      arg = (match[7] ? arg.substring(0, match[7]) : arg);
+      break;
+    case "x":
+      arg = parseInt(arg, 10).toString(16);
+      break;
+    case "X":
+      arg = parseInt(arg, 10).toString(16).toUpperCase();
+      break;
+  }
+  if (re.json.test(match[8])) {
+    return arg;
+  }
+
+  if (re.number.test(match[8]) && (!isPositive || match[3])) {
+    sign = isPositive ? "+" : "-";
+    arg = arg.toString().replace(re.sign, "");
+  } else {
+    sign = "";
+  }
+
+  if (match[4]) {
+    padCharacter = match[4] === "0" ? "0" : match[4].charAt(1);
+  } else {
+    padCharacter = " ";
+  }
+
+  padLength = match[6] - (sign + arg).length;
+
+  if (match[6] && padLength > 0) {
+    pad = strRepeat(padCharacter, padLength);
+  } else {
+    pad = "";
+  }
+
+  if (match[5]) {
+    return sign + arg + pad;
+  } else if (padCharacter === "0") {
+    return sign + pad + arg;
+  }
+
+  return pad + sign + arg;
+};
+
 stringFormat.parse = function (fmt) {
   let _fmt = fmt, match = [], parseTree = [], argNames = 0;
+
+  // Most of the formatting patterns used in DevTools are simple patters that contain a
+  // single %S placeholders. Keep track of the % placeholders found in the template to
+  // detect simple patterns.
+  let placeholders = [];
   while (_fmt) {
     if ((match = re.text.exec(_fmt)) !== null) {
       parseTree[parseTree.length] = match[0];
     } else if ((match = re.modulo.exec(_fmt)) !== null) {
       parseTree[parseTree.length] = "%";
     } else if ((match = re.placeholder.exec(_fmt)) !== null) {
       if (match[2]) {
         argNames |= 1;
@@ -219,21 +257,33 @@ stringFormat.parse = function (fmt) {
       } else {
         argNames |= 2;
       }
       if (argNames === 3) {
         throw new Error("[stringFormat] mixing positional and named placeholders is " +
           "not supported");
       }
       parseTree[parseTree.length] = match;
+      placeholders[placeholders.length] = match;
     } else {
       throw new SyntaxError("[stringFormat] unexpected placeholder");
     }
     _fmt = _fmt.substring(match[0].length);
   }
+
+  if (placeholders.length === 1) {
+    let m = placeholders[0];
+    let isString = m[8] === "S";
+    let hasPadding = m[6];
+    // A simple pattern is a pattern with one placeholder of string type, and no padding.
+    if (isString && !hasPadding) {
+      return SIMPLE_STRING_PATTERN;
+    }
+  }
+
   return parseTree;
 };
 
 var preformattedPadding = {
     "0": ["", "0", "00", "000", "0000", "00000", "000000", "0000000"],
     " ": ["", " ", "  ", "   ", "    ", "     ", "      ", "       "],
     "_": ["", "_", "__", "___", "____", "_____", "______", "_______"],
 };