819945 - Global searches choke when a source takes too long to fetch

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Victor Porof [:vporof][:vp]]

Go to https://tbpl.mozilla.org/?tree=Fx-Team and try performing a global search. The number of chars to search doesn't matter. Sometimes, if at least one source takes a very long time to fetch, the whole process of searching + displaying results waits for it to finish, which results many seconds of delay until results are shown.

We need to either
a). Display the search results as soon as each source's text becomes available
or
b). Timeout when a source takes too long to fetch and display all the other results immediately

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

github.com is an even better example of this. Maybe this is a good opportunity to implement lazy source fetching in the frontend, by only requesting the amount of text that fits in the visible part of the editor (plus an extra buffer for better scrolling performance).

Comment 2

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #1)
> github.com is an even better example of this. Maybe this is a good
> opportunity to implement lazy source fetching in the frontend, by only
> requesting the amount of text that fits in the visible part of the editor
> (plus an extra buffer for better scrolling performance).

Indeed. However, there's not really much benefit with being restricted to searching the contents in the visible part of the editor, especially when performing global searches. But incremental searching would be awesome. It's essentially a more granular approach to my a) in comment 0. But it's not a trivial thing to refactor/implement, so I'd settle to a variation of b) for now, at least until bug 786832 is fixed.

b'). Try and fetch each source's text. If a source takes more than 50ms to fetch, skip it when building the search results.

Comment 3

[Victor Porof [:vporof][:vp]]

Attachment: v1

Went along with the implementation described in comment 2; at least until bug 786832 is fixed, this will suffice. Behavior should be already verified by the numerous existing tests.

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 694334 [details] [diff] [review]
v1

Review of attachment 694334 [details] [diff] [review]:
-----------------------------------------------------------------

OK. Can you think of a case where the request will always time out and the resulting script will never be fetched without the user manually selecting it from the list?

::: browser/devtools/debugger/debugger-controller.js
@@ +1183,2 @@
>     */
> +  getText: function SS_getText(aSource, aCallback, aTimeout) {

aTimeout makes me think of milliseconds. How about aOnTimeout?

Comment 5

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #4)
> Comment on attachment 694334 [details] [diff] [review]
> v1
> 
> Review of attachment 694334 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> OK. Can you think of a case where the request will always time out and the
> resulting script will never be fetched without the user manually selecting
> it from the list?
> 

Not really. Even though the timeout occurs, fetching is still happening in the background, so the next time you search (or press RETURN), the results will be shown in the UI.

> ::: browser/devtools/debugger/debugger-controller.js
> @@ +1183,2 @@
> >     */
> > +  getText: function SS_getText(aSource, aCallback, aTimeout) {
> 
> aTimeout makes me think of milliseconds. How about aOnTimeout?

Ok.

Comment 6

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/3cc5ceca28af

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/mozilla-central/rev/3cc5ceca28af