Global searches choke when a source takes too long to fetch

RESOLVED FIXED in Firefox 20

Status

()

Firefox
Developer Tools: Debugger
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vporof, Assigned: vporof)

Tracking

unspecified
Firefox 20
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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
(Assignee)

Updated

5 years ago
Summary: Global searches choke when a source take too long to fetch → Global searches choke when a source takes too long to fetch
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).
(Assignee)

Comment 2

5 years ago
(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.
(Assignee)

Updated

5 years ago
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Priority: -- → P3
(Assignee)

Comment 3

5 years ago
Created attachment 694334 [details] [diff] [review]
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.
Attachment #694334 - Flags: review?(past)
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?
Attachment #694334 - Flags: review?(past) → review+
(Assignee)

Comment 5

5 years ago
(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.
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/3cc5ceca28af
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/3cc5ceca28af
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 20
You need to log in before you can comment on or make changes to this bug.