825135 - Live search too slow on large inboxes

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::E-Mail, defect, P2)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

STR
 (1) Add large email account
 (2) Go to search box
 (3) Enter 5 or 10 characters relatively quickly

The keyboard responsiveness gets bad and characters that are entered appear in the input box about 1 once every 10-15 seconds.

Suspect an explicit "Search" button, while ugly, would be an overall better UX.

bb? -> radar

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Important disclaimer: This was crash-landed functionality with minimal test coverage because of that whole first terrifying (alleged) feature freeze where I crash-landed several features.

Useful contextual notes:

- Search triggering does not currently wait for input to quiesce.
- The back-end still runs in the page's (main) thread, not on a worker (bug 814257).
- We ask for 15 messages to start and then keep asking for another 15 until we reach our target goal.  Although this is an asynchronous process, we do not use setTimeout to throttle things, so it's not surprising we might dominate the thread.  We could use a timeout either from the get-go or after allowing a small burst of activity covering some constant number of messages.
- A result of the "go until we find enough matches" strategy is that searches that don't match anything will be brutal because we will scan all synchronized messages in the folder.  If you are searching for the word "the", you will probably be fine.

VERY IMPORTANT NOTES:

- There appears to be a serious bug in that although we try to cancel the search when we issue a new one, this does not have any effect until we actually encounter a hit, at which point a null deref will kill us.  So if you manage to type something that will never match anything in your folder, that search has to traverse the entire folder before it will terminate.
- The search mechanism does not trigger cache flushing, so memory usage could get bad on large folders when we fail to match things.  There is some complexity/risk in that we don't hold a mutex while issuing the search right now.  We should probably just hold the mutex.  The SearchSlice is not currently registered in FolderStorage._slices so it will not inhibit discards.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I did notice a crash when entering a garbage search term.

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

Triage: BB-, tracking-b2g18+. it works if i type slowly. not ideal but usable

Comment 4

[James Lal [:lightsofapollo]]

To clarify, I think what we need here is a standard timeout before issuing the real backend request to search... ( think autocomplete )

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Andrew confirmed via IRC today that this is still a problem.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

NB: The patches landed for bug 838843 and bug 882917 and some subsequent changes for bug 796474 and its vscroll implementation have addressed a lot of things.  I'll call them out explicitly:

(In reply to Andrew Sutherland (:asuth) from comment #1)
> Important disclaimer: This was crash-landed functionality with minimal test
> - Search triggering does not currently wait for input to quiesce.

Still happens / not yet fixed/improved.

> - The back-end still runs in the page's (main) thread, not on a worker (bug
> 814257).

No longer true.  Searches happen on the worker thread.

> - We ask for 15 messages to start and then keep asking for another 15 until
> we reach our target goal.  Although this is an asynchronous process, we do
> not use setTimeout to throttle things, so it's not surprising we might
> dominate the thread.  We could use a timeout either from the get-go or after
> allowing a small burst of activity covering some constant number of messages.

No change in logic; it's debatable whether setTimeout is appropriate since we're on a worker and there definitely is a tradeoff.  Since this happens on a worker and there are likely to be I/O dependencies, we shouldn't peg the CPU but will generate significant load.

> - A result of the "go until we find enough matches" strategy is that
> searches that don't match anything will be brutal because we will scan all
> synchronized messages in the folder.  If you are searching for the word
> "the", you will probably be fine.

"brutal" is now debatable, but this being the worst-case scenario still follows.

> VERY IMPORTANT NOTES:
> 
> - There appears to be a serious bug in that although we try to cancel the
> search when we issue a new one, this does not have any effect until we
> actually encounter a hit, at which point a null deref will kill us.  So if
> you manage to type something that will never match anything in your folder,
> that search has to traverse the entire folder before it will terminate.

Addressed by checking that we still have a _bridgeHandle.

> - The search mechanism does not trigger cache flushing, so memory usage
> could get bad on large folders when we fail to match things.  There is some
> complexity/risk in that we don't hold a mutex while issuing the search right
> now.  We should probably just hold the mutex.  The SearchSlice is not
> currently registered in FolderStorage._slices so it will not inhibit
> discards.

Addressed by having block loading automatically trigger flushes on a timer.  Search slices still do not impact caching and do not hold the mutex.

Comment 7

[BMO Automation]

Firefox OS is not being worked on