1765442 - Large lag from identifyAutofillFields on page with many forms

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, defect, P1)

Description

[matze.kind]

Attachment: Firefox 2022-04-20 00.10 profile.json.gz

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:99.0) Gecko/20100101 Firefox/99.0

Steps to reproduce:

I opened a page using many form fields with labels (in my case a lage json-editor https://github.com/json-editor/json-editor ) and entered some data in text fields / selected diffrent options of a <select>

UA: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:99.0) Gecko/20100101 Firefox/99.0
OS: Darwin 21.4.0 Darwin Kernel Version 21.4.0: Fri Mar 18 00:45:05 PDT 2022; root:xnu-8020.101.4~15/RELEASE_X86_64
Device: Macbook Pro 15' 2017 i7 16GB

Actual results:

At some point (just after entering some string or switching an <select> to a diffrent option), the whole page (JS) freezes for 20-40 seconds. (see profile attached)
The page was deployed more than 50 days ago, so no dependency change there that caused this. Until a few days ago I never encountered this kind of lag after inital initalization of the editor, so I guess this was introduced in FF 99

Profiling shows 35 seconds (86%) spent in identifyAutofillFields -> ... -> findLabelElements

Expected results:

Like with previous versions of FF, the autofill machanic should not block the page noticable for the user, especially when the DOM mostly didn't change

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Form Controls' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

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

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

For more information, please visit auto_nag documentation.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Sorry for the lag here.

Shareable profile for convenience: https://share.firefox.dev/37D6rGc

This looks pretty bad... Dimi can you take a look? You seem to be the last one that touched that relevant code. Though it seems significant changes happened in the 100 release. Reporter, is this still happening with Firefox 100?

Comment 4

[Dimi Lee [:dimi]]

Firefox 100 did change a lot of stuff, but the flow is basically the same, I guess this issue will still be reproducible in 100.
The profile shows that we spend a lot of time extracting labels from elements. This is for our heuristics to determine whether a field is a credit card field or an address field. The poor performance on form in many fields with labels is actually not surprising to me, I can see there is a lot of room to improve.

Hi matze, could you share with me the page you reproduce this issue, then I can make sure we improve our heuristic in the right direction.

Comment 5

[Dimi Lee [:dimi]]

Attachment: Bug 1765442 - Store the reference of HTMLLabelElement.control when generating label maps r=sgalich!

Accessing control via label.control requires a call to native HTMLLabelElement::GetLabeledElement.
And for unmapped label case, we call
this._unmappedLabels.filter(label => label.control == element);
for every input element in the form. This means if a form has 10 labels
and 10 elements, a identifyAutofillFields call to the form may invoke up to 100 (10x10)
HTMLLabelElement::GetLabeledElement call.

This patch fixes this issue by storing the reference of the control elements in
this._unmappedLabelControls to reduce the number of HTMLLabelElement::GetLabeledElement calls
(10 HTMLLabelElement::GetLabeledElement calls in the foregoing example).

Comment 6

[Dimi Lee [:dimi]]

I think I've found the root cause, clear the needinfo.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf51f1125d46
Store the reference of HTMLLabelElement.control when generating label maps r=sgalich

Comment 8

[Atila Butkovits]

Backed out for causing failures at test_autocomplete_off_on_inputs.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/84b9f42369c1f7849288ea7acda5faeea4917704

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=cf51f1125d465bf2bf792feb5ef4b1591e828256&selectedTaskRun=K5rYl7jxS4qbyetPrFHfYw.0

Failure log: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=cf51f1125d465bf2bf792feb5ef4b1591e828256&selectedTaskRun=K5rYl7jxS4qbyetPrFHfYw.0

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb3d80b0b483
Store the reference of HTMLLabelElement.control when generating label maps r=sgalich

Comment 10

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/cb3d80b0b483