1940667 - 30s of jank on parent-process doing something around getFixupURIInfo and ContentAreaDropListener.sys.mjs (resource://gre/modules/URIFixup.sys.mjs:278:18)

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Mayank Bansal]

Attachment: testcase.html

Go to http://html2hiccup.buttercloud.com/
Copy-paste the attached HTML text into the input box
Wait for the output to appear
The output will appear preselected or something.
Now select all the output text (Ctrl +A )
Drag the text slightly just enough so that the cursor changes its shape. Then drop it.

Profile: https://share.firefox.dev/4gGKPa6 / https://share.firefox.dev/40oBPAN
20s in processing stuff, 10s in CC

Comment 1

[Mayank Bansal]

Can repro on a build from Jan2021(!), so not a new regression. But very surprising that this is an old issue that hasnt surfaced yet.

Comment 2

[Marco Bonardo [:mak] (Away 10-21 August)]

The stack looks like the same as bug 1838723, the underlying cause is the same.
GetSubmission gets passed over a huge string to build a search for, it should just give up due to excessive length.

Comment 3

[Mark Banner (:standard8)]

I took a look at this as I was wondering where GetSubmission is expensive (there's not too much we can do about that).

The test case is effectively dragging (and then dropping) more than 400,000 lines of data similar to:

[:div  
 [:span "Hello"]
 [:span "Hello"]
 [:span "Hello"]
 [:span "Hello"]
 [:span "Hello"]

ContentAreaDropListener._addLinksFromItem is iterating over all 400,000 lines and running Services.uriFixup.getFixupURIInfo on them to see if any of them are links.

This was added in bug 1435910 (bug 1435910 comment 8 describes the plans), so I suspect it is an old regression from that.

There's potentially a few things we could do to improve this:

• Avoid looking up keywords (which would avoid the call to engine.getSubmission). This is unnecessary, since if a submission matches, that gets put into info.preferredURI which is never read.
  • On my machine this reduces the time from ~14s down to ~9s.
  • The only downside I can see is that we'll log a failure to the console due to the XPCOM call throwing.
• Marco suggested adding a flag which would skip converting single words when !isDomainKnown().
• We could also bail out of ContentAreaDropListener after N failures, hence assuming it isn't a list of URIs.

Comment 4

[Mayank Bansal]

Two points:

1. I never dropped the text. I just kept it hovering. I only drop it back into the output-box.
2. I never hovered the text over the URL bar.
3. You dont need to hover text over the input-box. In the output box, drag the text just enough that the cursor changes its shape. Then release the text.
4. Profile: https://share.firefox.dev/4h5GpJK

Maybe the long time in CC can be improved too?

Comment 5

[BugBot [:suhaib / :marco]]

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

:arai, since you are the author of the regressor, bug 1435910, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 6

[Mark Banner (:standard8)]

(In reply to Mayank Bansal from comment #4)

Maybe the long time in CC can be improved too?

I think avoiding unnecessary work, or stopping early should mean that we're not creating lots of objects, which will in turn mean that the CC time will be reduced.

Comment 7

[Marco Bonardo [:mak] (Away 10-21 August)]

(In reply to Mark Banner (:standard8) from comment #3)

• Marco suggested adding a flag which would skip converting single words when !isDomainKnown().

This is in case we disable lookup, then it may end up transforming mozilla into http://mozilla/ a flag could just make it throw if the domain is not in the allow list, and isDomainKnown can be used for that.

Comment 8

[Frank Doty [:fdoty]]

adding a perf impact review as 30sec of jank is quite rough. This triage should give us a better idea of severity

Comment 9

[Bas Schouten (:bas.schouten)]

The Performance Impact Calculator has determined this bug's performance impact to be low. If you'd like to request re-triage, you can reset the Performance Impact flag to "?" or needinfo the triage sheriff.

Platforms: Windows
Impact on browser: Causes noticeable jank
Configuration: Rare

Comment 10

[Mayank Bansal]

Doesnt repro anymore.
I am guessing this got fixed by bug 2002920
Marking as fixed.