Draft issue to file on mozilla-services/fathom-login-forms to improve model performance

Improve ruleset performance with mostly low hanging fruit.md

Since Bug 1595244 landed yesterday (based on the model at bff6995c), we have started to get some telemetry regarding how long it takes to show the password generation UI. We also did some profiling this afternoon. Taking both of these together, we think it would be prudent to make some relatively straightforward changes to the ruleset to improve its performance.

The following numbers are based on profiling Nightly, taking an average of 5 runs for a locally hosted registration page. Taken together, these sets of changes would speed up the model by about 42%, and there may be more low hanging fruit as well.

The diff below would speed up the model by about 25%. This would not require any retraining.

diff --git a/toolkit/components/passwordmgr/NewPasswordModel.jsm b/toolkit/components/passwordmgr/NewPasswordModel.jsm
--- a/toolkit/components/passwordmgr/NewPasswordModel.jsm
+++ b/toolkit/components/passwordmgr/NewPasswordModel.jsm
@@ -27,6 +27,8 @@ const {
   clusters: { euclidean },
 } = fathom;
 
+const elementToAllAnchorElementDescendants = new WeakMap();
+
 /**
  * ----- Start of model -----
  *
@@ -95,6 +97,10 @@ const password1Or2Regex = /password1|pas
 const passwordyRegex = /pw|pwd|passwd|pass/i;
 const loginRegex = /login|Войти|sign in|ورود|登录|Přihlásit se|Авторизоваться|signin|log in|sign\/in|sign-in|entrar|ログインする|로그인/i;
 const registerButtonRegex = /create account|Zugang anlegen|Angaben prüfen|Konto erstellen|register|sign up|create an account|create my account|ثبت نام|登録|Cadastrar|Зарегистрироваться|Bellige alynmak/i;
+const passwordAndPasswordyRegex = new RegExp(
+  passwordRegex.source + "|" + passwordyRegex.source,
+  "i"
+);
 
 function makeRuleset(coeffs, biases) {
   /**
@@ -239,11 +245,16 @@ function makeRuleset(coeffs, biases) {
   }
 
   function hasAnchorMatchingPredicateWithinElement(element, matchingPredicate) {
-    if (element !== null) {
-      const anchors = Array.from(element.querySelectorAll("a"));
-      return anchors.some(matchingPredicate);
+    if (element === null) {
+      return false;
     }
-    return false;
+
+    let anchors = elementToAllAnchorElementDescendants.get(element);
+    if (!anchors) {
+      anchors = Array.from(element.querySelectorAll("a"));
+      elementToAllAnchorElementDescendants.set(element, anchors);
+    }
+    return anchors.some(matchingPredicate);
   }
 
   function forgotPasswordButtonWithinElement(element) {
@@ -383,7 +394,7 @@ function makeRuleset(coeffs, biases) {
           forgotPasswordInAnchorPropertyWithinElement(
             "href",
             fnode.element.form,
-            new RegExp(passwordRegex.source + "|" + passwordyRegex.source, "i"),
+            passwordAndPasswordyRegex,
             forgotPasswordHrefRegex
           )
         ),
@@ -424,7 +435,7 @@ function makeRuleset(coeffs, biases) {
           forgotPasswordInAnchorPropertyWithinElement(
             "href",
             fnode.element.ownerDocument,
-            new RegExp(passwordRegex.source + "|" + passwordyRegex.source, "i"),
+            passwordAndPasswordyRegex,
             forgotPasswordHrefRegex
           )
         ),

The diff below would speed the model up by an additional 17%. This may require retraining and/or affect the error rate.

diff --git a/toolkit/components/passwordmgr/NewPasswordModel.jsm b/toolkit/components/passwordmgr/NewPasswordModel.jsm
--- a/toolkit/components/passwordmgr/NewPasswordModel.jsm
+++ b/toolkit/components/passwordmgr/NewPasswordModel.jsm
@@ -121,7 +121,7 @@ function makeRuleset(coeffs, biases) {
     const labels = element.labels;
     // TODO: Should I be concerned with multiple labels?
     if (labels !== null && labels.length) {
-      return regex.test(labels[0].innerText);
+      return regex.test(labels[0].textContent);
     }
 
     // Check element.aria-labelledby
@@ -131,24 +131,24 @@ function makeRuleset(coeffs, biases) {
         .split(" ")
         .map(id => element.ownerDocument.getElementById(id));
       if (labelledBy.length === 1) {
-        return regex.test(labelledBy[0].innerText);
+        return regex.test(labelledBy[0].textContent);
       } else if (labelledBy.length > 1) {
         return regex.test(
-          min(labelledBy, node => euclidean(node, element)).innerText
+          min(labelledBy, node => euclidean(node, element)).textContent
         );
       }
     }
 
     const parentElement = element.parentElement;
-    // Check if the input is in a <td>, and, if so, check the innerText of the containing <tr>
+    // Check if the input is in a <td>, and, if so, check the textContent of the containing <tr>
     if (parentElement.tagName === "TD") {
       // TODO: How bad is the assumption that the <tr> won't be the parent of the <td>?
-      return regex.test(parentElement.parentElement.innerText);
+      return regex.test(parentElement.parentElement.textContent);
     }
 
-    // Check if the input is in a <dd>, and, if so, check the innerText of the preceding <dt>
+    // Check if the input is in a <dd>, and, if so, check the textContent of the preceding <dt>
     if (parentElement.tagName === "DD") {
-      return regex.test(parentElement.previousElementSibling.innerText);
+      return regex.test(parentElement.previousElementSibling.textContent);
     }
     return false;
   }
@@ -170,12 +170,12 @@ function makeRuleset(coeffs, biases) {
       previousElementSibling !== null &&
       previousElementSibling.tagName === "LABEL"
     ) {
-      return regex.test(previousElementSibling.innerText);
+      return regex.test(previousElementSibling.textContent);
     }
 
     const nextElementSibling = element.nextElementSibling;
     if (nextElementSibling !== null && nextElementSibling.tagName === "LABEL") {
-      return regex.test(nextElementSibling.innerText);
+      return regex.test(nextElementSibling.textContent);
     }
 
     const closestLabelWithinForm = closestSelectorElementWithinElement(
@@ -186,7 +186,7 @@ function makeRuleset(coeffs, biases) {
     return containsRegex(
       regex,
       closestLabelWithinForm,
-      closestLabelWithinForm => closestLabelWithinForm.innerText
+      closestLabelWithinForm => closestLabelWithinForm.textContent
     );
   }
 
@@ -250,10 +250,10 @@ function makeRuleset(coeffs, biases) {
     if (element !== null) {
       const buttons = Array.from(element.querySelectorAll("button"));
       return buttons.some(button => {
-        const innerText = button.innerText;
+        const textContent = button.textContent;
         return (
-          passwordRegex.test(innerText) &&
-          forgotPasswordInnerTextRegex.test(innerText)
+          passwordRegex.test(textContent) &&
+          forgotPasswordInnerTextRegex.test(textContent)
         );
       });
     }
@@ -282,7 +282,7 @@ function makeRuleset(coeffs, biases) {
       return buttons.some(button => {
         return (
           stringRegex.test(button.value) ||
-          stringRegex.test(button.innerText) ||
+          stringRegex.test(button.textContent) ||
           stringRegex.test(button.id)
         );
       });
@@ -369,7 +369,7 @@ function makeRuleset(coeffs, biases) {
         type("new"),
         score(fnode =>
           forgotPasswordInAnchorPropertyWithinElement(
-            "innerText",
+            "textContent",
             fnode.element.form,
             passwordRegex,
             forgotPasswordInnerTextRegex
@@ -410,7 +410,7 @@ function makeRuleset(coeffs, biases) {
         type("new"),
         score(fnode =>
           forgotPasswordInAnchorPropertyWithinElement(
-            "innerText",
+            "textContent",
             fnode.element.ownerDocument,
             passwordRegex,
             forgotPasswordInnerTextRegex