1704167 - "Find In Page" performance regression in some cases

Status: RESOLVED FIXED

(Core :: Find Backend, defect)

Description

[tgn-ff]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101 Firefox/89.0

Steps to reproduce:

1. Open this link of a bugzilla search result.

2. Search for render. The space at the start of the search parameter is important.

Actual results:

The initial search results and the subsequent findings of the next occurrences take several seconds. The "tab spinner" shows up if you switch to another tab while the search is in progress.

Expected results:

It should be as fast as the results without the space at the start of the search param.

Mozregression points to Bug1659897: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f160d1442e60f9a5dcf8d427bf6f9cb5a341137f&tochange=dd7fb8f29da26ebcd74febb515588efe97e964fc

Performance profile with the Firefox Platform preset (during a few "find next" button presses): https://share.firefox.dev/3dRfEK8

Comment 1

[tgn-ff]

At step 2 the <space> at the beginning isn't shown inside the code tag. You have to enter "<space>render" at that step.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Thanks for the report, I can reproduce. Will take a look when I find some time.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1704167 - Don't rewind if we're still at the beginning of the pattern. r=jfkthame

This can happen with whitespace, and can cause us to scan whitespace
exponentially. Should be straight-forward and have no behavior change.

This restores the performance characteristics of the findbar before
the regressing bug.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1704167 - When a pattern ends in whitespace, try to collapse adjacent white-space. r=jfkthame

This I'm not 100% sure. Should be harmless, but it's a bit subtle.
Maybe we should special-case it with "we're at the beginning of the
pattern"? Reasoning below:

The previous patch restores the performance of the original test-case.

However, if you go to the reduced test-case and try to type " re" in the
findbar, we still take a long time. The reason for that is not that the
previous patch is not effective, but that the findbar sends find
requests as soon as you type, and thus we end up with a request to find
" ", which matches a gazillion spaces in the page and causes us to use
tons of memory and time. Finding " re" is actually super-fast :-)

This fixes it, but it is a bit subtle, so thoughts? Perhaps the findbar
should wait a bit to perform the search before sending a query for " "
instead or something? But I'd rather make it fast.

Depends on D111633

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac196dc43025
Don't rewind if we're still at the beginning of the pattern. r=jfkthame

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64b1938f0ed6
When a pattern ends in whitespace, try to collapse adjacent white-space. r=jfkthame

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ac196dc43025
https://hg.mozilla.org/mozilla-central/rev/64b1938f0ed6