1998089 - 0.39% installer size () regression on Tue October 28 2025

Status: RESOLVED WONTFIX

(Firefox :: Translations, defect)

Description

[Treeherder Bug Filer]

Perfherder has detected a build_metrics performance regression from push c93ccc131fe16452f475e406db2117fab785a6ff. As author of one of the patches included in that push, we need your help to address this regression.

Please acknowledge, and begin investigating this alert within 3 business days, or the patch(es) may be backed out in accordance with our regression policy. Our guide to handling regression bugs has information about how you can proceed with this investigation.

If you have any questions or need any help with the investigation, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
0.39%	installer size	win64-nightlyasrelease	nightly nightly-as-release	127,612,251.08 -> 128,105,584.08

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

You can run all of these tests on try with ./mach try perf --alert 47265

The following documentation link provides more information about this command.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1861698

Comment 2

[Erik Nordin [:nordzilla]]

I can't think of anything in this patch that would affect installer size.

:afinder, do you think this was misdiagnosed?

Comment 3

[Alex Finder]

(In reply to Erik Nordin [:nordzilla] from comment #2)

I can't think of anything in this patch that would affect installer size.

:afinder, do you think this was misdiagnosed?

Hi Erik! The graph shows a clear regression between revision c93ccc131fe1 and the one before it, with no gap between the two revisions. Perfcompare also shows a high confidence on the regression between the before and after revisions. I think it's very unlikely the culprit was incorrectly identified.
Let me know via :ni if I can help with anything else.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1861698

Comment 5

[Alex Finder]

(In reply to Alex Finder from comment #3)

(In reply to Erik Nordin [:nordzilla] from comment #2)

I can't think of anything in this patch that would affect installer size.

:afinder, do you think this was misdiagnosed?

Hi Erik! The graph shows a clear regression between revision c93ccc131fe1 and the one before it, with no gap between the two revisions. Perfcompare also shows a high confidence on the regression between the before and after revisions. I think it's very unlikely the culprit was incorrectly identified.
Let me know via :ni if I can help with anything else.

Forgot to add ni to Erik.

Comment 6

[Erik Nordin [:nordzilla]]

Analysis

Full Diff

I'm looking at the diff of my commits here:

❯ git diff --stat 5ef169b4891fc4c7947f0064f63678ec205b19c7^..8cd5d52a46f898a44589239a49ec90aec22a6e18

 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_auto_translate.js            | 18 ++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_basics.js                    | 18 ++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_open_panel.js                | 18 ++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_panel_auto_offer.js          | 18 ++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_panel_auto_offer_settings.js | 18 ++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_retranslate.js               | 18 ++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_switch_languages.js          | 36 ++++++++++++++++++++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_translation_failure.js       | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_translation_request.js       | 36 ++++++++++++++++++++++++++++++++++++
 browser/components/translations/tests/browser/browser_translations_full_page_telemetry_unsupported_lang.js          |  7 +++++++
 toolkit/components/translations/TranslationsTelemetry.sys.mjs                                                       | 43 +++++++++++++++++++++++++++++++++++++++++++
 toolkit/components/translations/actors/TranslationsChild.sys.mjs                                                    | 23 +++++++++++++++++++++++
 toolkit/components/translations/actors/TranslationsParent.sys.mjs                                                   | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
 toolkit/components/translations/metrics.yaml                                                                        | 43 +++++++++++++++++++++++++++++++++++++++++++
 toolkit/components/translations/tests/browser/shared-head.js                                                        |  4 ++--
 15 files changed, 437 insertions(+), 32 deletions(-)

---

Ignore Tests

Let's ignore anything in the tests directory, because that certainly isn't it.

❯ git diff --stat 5ef169b4891fc4c7947f0064f63678ec205b19c7^..8cd5d52a46f898a44589239a49ec90aec22a6e18 -- ':(exclude)*tests*'

 toolkit/components/translations/TranslationsTelemetry.sys.mjs     | 43 +++++++++++++++++++++++++++++++++++++++++++
 toolkit/components/translations/actors/TranslationsChild.sys.mjs  | 23 +++++++++++++++++++++++
 toolkit/components/translations/actors/TranslationsParent.sys.mjs | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
 toolkit/components/translations/metrics.yaml                      | 43 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+), 30 deletions(-)

---

Ignore Telemetry

I've added plenty of telemetry metrics without any negative effect, so I'm assuming that isn't it, but I'll send a quick NI to :chutten just to confirm that this likely isn't it.

So that leaves just the TranslationsParent and TranslationsChild changes.

git diff --stat 5ef169b4891fc4c7947f0064f63678ec205b19c7^..8cd5d52a46f898a44589239a49ec90aec22a6e18 -- \
  ':(exclude)*tests*' \
  ':(exclude)*Telemetry*' \
  ':(exclude)*metrics.yaml'

 toolkit/components/translations/actors/TranslationsChild.sys.mjs  | 23 +++++++++++++++++++++++
 toolkit/components/translations/actors/TranslationsParent.sys.mjs | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
 2 files changed, 90 insertions(+), 30 deletions(-)

---

Remaining Diff

Here is the full diff of only those changes:

diff --git a/toolkit/components/translations/actors/TranslationsChild.sys.mjs b/toolkit/components/translations/actors/TranslationsChild.sys.mjs
index 7e3cc0e80dd0..0f1daa5e09c6 100644
--- a/toolkit/components/translations/actors/TranslationsChild.sys.mjs
+++ b/toolkit/components/translations/actors/TranslationsChild.sys.mjs
@@ -78,6 +78,29 @@ export class TranslationsChild extends JSWindowActorChild {
         this.#translatedDoc?.enterLazyTranslationsMode();
         return undefined;
       }
+      case "Translations:ExtractPageText": {
+        const { document } = this;
+        if (!document) {
+          return "";
+        }
+
+        const { sufficientLength } = data;
+
+        const encoder = Cu.createDocumentEncoder("text/plain");
+        encoder.init(
+          document,
+          "text/plain",
+          Ci.nsIDocumentEncoder.OutputBodyOnly |
+            Ci.nsIDocumentEncoder.SkipInvisibleContent |
+            Ci.nsIDocumentEncoder.AllowCrossShadowBoundary |
+            Ci.nsIDocumentEncoder.OutputForPlainTextClipboardCopy |
+            Ci.nsIDocumentEncoder.OutputDisallowLineBreaking |
+            Ci.nsIDocumentEncoder.OutputDropInvisibleBreak |
+            Ci.nsIDocumentEncoder.OutputLFLineBreak
+        );
+
+        return encoder.encodeToStringWithMaxLength(sufficientLength);
+      }
       case "Translations:TranslatePage": {
         if (this.#translatedDoc?.engineStatus === "error") {
           this.#translatedDoc.destroy();
diff --git a/toolkit/components/translations/actors/TranslationsParent.sys.mjs b/toolkit/components/translations/actors/TranslationsParent.sys.mjs
index a7e1ff26110a..9cfbc9b5dcf3 100644
--- a/toolkit/components/translations/actors/TranslationsParent.sys.mjs
+++ b/toolkit/components/translations/actors/TranslationsParent.sys.mjs
@@ -3554,7 +3554,8 @@ export class TranslationsParent extends JSWindowActorParent {
   }

   /**
-   * Uses the page extractor to identify the current page's language.
+   * Extracts a substring of visible text from the content document and
+   * runs it through the language detector to determine the page's language.
    *
    * @returns {Promise<DetectionResult>}
    */
@@ -3563,53 +3564,89 @@ export class TranslationsParent extends JSWindowActorParent {
       return this.languageState.detectedLanguages.identified;
     }

-    const actor =
-      this.browsingContext?.currentWindowGlobal?.getActor("PageExtractor");
-
-    if (!actor) {
-      throw new Error("Unable to get the PageExtractor actor.");
-    }
+    lazy.console.log(
+      "Beginning text extraction:",
+      this.browsingContext?.currentURI?.spec
+    );

-    const startTime = ChromeUtils.now();
+    const extractionStartTime = ChromeUtils.now();
+    const pageText = await this.sendQuery("Translations:ExtractPageText", {
+      sufficientLength: 4096,
+    });

-    // Manual profiling on 10 page loads of https://es.wikipedia.org/wiki/Felis_catus:
-    // -------------------------------------------------------------------------------
-    //
-    //   No limit: 2064 samples, 224/237/294 [min/med/max]ms (~85k code units)
-    // 8192 limit:  681 samples,  75/ 87/128 [min/med/max]ms
-    // 4096 limit:  457 samples,  51/ 55/ 97 [min/med/max]ms
-    // 2048 limit:  240 samples,  29/ 39/ 64 [min/med/max]ms
-    // 1024 limit:  142 samples,  19/ 28/ 58 [min/med/max]ms
-    //
-    // 2048 Code units feels like a decent length for performance and sample size.
-    const pageText = await actor.getText({ sufficientLength: 2048 });
     if (this.#isDestroyed) {
-      return { language: "", confident: false, languages: [] };
+      return { language: "en", confident: false, languages: [] };
     }

+    const extractionTime = ChromeUtils.now() - extractionStartTime;
+
+    lazy.console.debug(
+      `Extracted Page Text (${pageText.length} code units):\n\n`,
+      pageText
+    );
+
+    const extractionLog =
+      `Extracted ${pageText.length} code units of text in ` +
+      `${extractionTime.toFixed(3)} ms.`;
+
+    lazy.console.log(extractionLog);
+    ChromeUtils.addProfilerMarker(
+      "TranslationsParent",
+      { startTime: extractionStartTime, innerWindowId: this.innerWindowId },
+      extractionLog
+    );
+
+    const identificationStartTime = ChromeUtils.now();
     const result = await lazy.LanguageDetector.detectLanguage(pageText);
+
     if (this.#isDestroyed) {
-      return { language: "", confident: false, languages: [] };
+      return { language: "en", confident: false, languages: [] };
     }

-    const message =
-      `Identified page language as "${result.language}" ` +
-      `in ${((ChromeUtils.now() - startTime) / 1000).toFixed(3)} seconds: ` +
-      this.browsingContext?.currentURI?.spec;
+    const identificationTime = ChromeUtils.now() - identificationStartTime;
+    const identificationLog =
+      `Identified ${pageText.length} code units of text as "${result.language}" ` +
+      `in ${identificationTime.toFixed(3)} ms.`;

+    lazy.console.log(identificationLog);
     ChromeUtils.addProfilerMarker(
       "TranslationsParent",
-      { startTime, innerWindowId: this.innerWindowId },
-      message
+      { startTime: identificationStartTime, innerWindowId: this.innerWindowId },
+      identificationLog
+    );
+    ChromeUtils.addProfilerMarker(
+      "TranslationsParent",
+      { startTime: extractionStartTime, innerWindowId: this.innerWindowId },
+      "Total time to identify page language."
     );
-
-    lazy.console.debug("\nExtracted Page Text:\n\n", pageText);
-    lazy.console.log(message);

     if (pageText.length < TranslationsParent.#DOC_CONFIDENCE_THRESHOLD) {
       result.confident = false;
     }

+    const htmlLangAttribute =
+      this.languageState?.detectedLanguages?.htmlLangAttribute ?? null;
+    const identifiedLanguage = result.language;
+
+    TranslationsParent.telemetry().onIdentifyPageLanguage({
+      htmlLangAttribute,
+      identifiedLanguage,
+      langTagsMatch: htmlLangAttribute
+        ? lazy.TranslationsUtils.langTagsMatch(
+            htmlLangAttribute,
+            identifiedLanguage
+          )
+        : null,
+      isLangAttributeValid: htmlLangAttribute
+        ? lazy.TranslationsUtils.isLangTagValid(htmlLangAttribute)
+        : null,
+      extractedCodeUnits: pageText.length,
+      extractionTime,
+      identificationTime,
+      totalTime: extractionTime + identificationTime,
+      confident: result.confident,
+    });
+
     return result;
   }

Now, I know that adding lines to TranslationsChild can be a bit sensitive to performance, since the child actor is loaded in every content tab. But I wouldn't expect it to affect installer size.

I should also note that the intent of this patch is to move the page-extraction algorithm back to using nsIDocumentEncoder, which it has been using for years without issue.

In Bug 1967758 I moved away from nsIDocumentEncoder, but I did more performance testing, and realized that nsIDcoumentEncoder was still faster, so this patch moves it back.

(This all occurred within the same Nightly cycle.)

Bob, I'm sorry for the cold-call NI, but I know you are familiar with Windows. Feel free to redirect if needed.

I genuinely am not sure what in this diff would affect Windows installer size by 481.3 KB. Do you see anything?

Comment 7

[Bob Owen (:bobowen)]

(In reply to Erik Nordin [:nordzilla] from comment #6)
...

Bob, I'm sorry for the cold-call NI, but I know you are familiar with Windows. Feel free to redirect if needed.

I genuinely am not sure what in this diff would affect Windows installer size by 481.3 KB. Do you see anything?

I don't know much about mjs files, however I know that when jstutte landed bug 1995254, he found that the installer size improved.
No idea if that could affect mjs files, but I see TimeStamp::Now is being used.
The increase here seems too large for that though.
Looking at the graph, I think the change came before your commit:
https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=0&highlightCommonAlerts=0&highlightedRevisions=c93ccc131fe16452f475e406db2117fab785a6ff&replicates=0&series=autoland,1457010,1,2&timerange=2592000&zoom=1761641701127,1761706669740,127230870,128412145.07500441

Comment 8

[Chris H-C :chutten|RustWeek (Back May 25)]

We haven't yet heard of any hand-rolled set of metrics causing binary size explosions. Only when we adjust how metrics are handled in aggregate (e.g. all events in bug 1932617) can we see detectable changes in either direction. Your intuition matches mine: adding instrumentation ought not to matter (much).

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:gregtatum, could you have a look please?

For more information, please visit BugBot documentation.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

It has been over 7 days with no activity on this performance regression.

:nordzilla, since you are the author of the regressor, bug 1861698, which triggered this performance alert, could you please provide a progress update?

If this regression is something that fixes a bug, changes the baseline of the regression metrics, or otherwise will not be fixed, please consider closing it as WONTFIX. See this documentation for more information on how to handle regressions.

For additional information/help, please needinfo the performance sheriff who filed this alert (they can be found in comment #0), or reach out in #perftest, or #perfsheriffs on Element.

For more information, please visit BugBot documentation.

Comment 11

[Erik Nordin [:nordzilla]]

Attachment: image.png

The installer size seems to be fairly volatile.

It's already dropped back down, and then gone back up since this regression at the beginning of October.

I don't think that this regression appears to be outside of the range of normal fluctuations, based on my viewing of this data.

---

Also, as I mention above, I genuinely don't know what about my patch would have affected the installer size.

Given that the fluctuation is not very large, and that there isn't a clear culprit, I'm inclined to say that it's not worth investigating this further.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

A comment containing a reason for why the performance regression was resolved as WONTFIX could not be found. It should provided when the status of the bug is changed.

:RyanVM, since you resolved this bug, could you provide a comment explaining the resolution? If one has already been provided, this needinfo can be ignored/removed.

If you need additional information/help, reach out in #perftest, or #perfsheriffs on Element.

For more information, please visit BugBot documentation.