593893 - Consider merging MatchAutoCompleteFunction::findOnBoundary and findAnywhere

Status: NEW

(Toolkit :: Places, defect, P5)

Description

[Justin Lebar (not reading bugmail)]

MatchAutoCompleteFunction::findOnBoundary and findAnywhere do almost the same thing.  findOnBoundary gets to skip over some characters, but in my testing, this doesn't make it much faster than findAnywhere.

As I understand it, the location bar first searches for matches with findOnBoundary, and if it doesn't find enough, it retries using findAnywhere.  We might be able to make this faster by merging findOnBoundary and findAnywhere into a new function which determines whether:

  * the needle doesn't appear in the haystack,
  * the needle appears at a word boundary, or
  * the needle appears somewhere other than a word boundary.

Comment 1

[Shawn Wilsher :sdwilsh]

(In reply to comment #0)
> MatchAutoCompleteFunction::findOnBoundary and findAnywhere do almost the same
> thing.  findOnBoundary gets to skip over some characters, but in my testing,
> this doesn't make it much faster than findAnywhere.
It's not meant to be an optimization in speed, but rather a usability win.  Users are more likely to be searching at the start of a word, which could be camelCase.

Comment 2

[Justin Lebar (not reading bugmail)]

(In reply to comment #1)
> It's not meant to be an optimization in speed, but rather a usability win. 
> Users are more likely to be searching at the start of a word, which could be
> camelCase.

Understood.  My point is just I think it's plausible that we could combine the two functions such that the new one doesn't run much slower than findOnBoundary, and that we could thus get rid of the pause between seeing the findOnBoundary and the findAnywhere results (and make the findAnywhere results show up faster when there are no findOnBoundary results).

Comment 3

[Shawn Wilsher :sdwilsh]

(In reply to comment #2)
> Understood.  My point is just I think it's plausible that we could combine the
> two functions such that the new one doesn't run much slower than
> findOnBoundary, and that we could thus get rid of the pause between seeing the
> findOnBoundary and the findAnywhere results (and make the findAnywhere results
> show up faster when there are no findOnBoundary results).
By caching them?  Not sure how I feel about that yet.  It's been a pretty dumb SQL function to date, but that would make it rather complicated.  I'd need more details on how you plan on doing it I think.

Comment 4

[Justin Lebar (not reading bugmail)]

(In reply to comment #3)
> By caching them?  Not sure how I feel about that yet.  It's been a pretty dumb
> SQL function to date, but that would make it rather complicated.  I'd need more
> details on how you plan on doing it I think.

Well, I don't really have a plan at the SQL level; I'd been looking at this from the level of spending less time in the two hot functions in MatchAutoCompleteFunctions.

But presumably there's a query which does something like

  SELECT TOP 50 record.fields
  WHERE {findOnBoundary,findAnywhere}(token, record.{title,url})
  ORDER BY frecency

If we had a findSomewhere(tok, str) function which returned 2 if tok was found on a word boundary, 1 if it was found anywhere, and 0 otherwise then maybe we could write

  SELECT TOP 50 record.fields
  WHERE findSomewhere(token, record.{title,url}) > 0
  ORDER BY findSomewhere(token, record.{title,url}), frecency

to get the same results we get now.  Maybe?  I haven't looked any of code, and I'm completely unfamiliar with how SQLite works -- I've only used SQLHeavy -- so perhaps this is a Dumb Idea.

Comment 5

[Shawn Wilsher :sdwilsh]

(In reply to comment #4)
>   SELECT TOP 50 record.fields
>   WHERE findSomewhere(token, record.{title,url}) > 0
>   ORDER BY findSomewhere(token, record.{title,url}), frecency
I think that this might end up calling our function twice, but not sure.

Comment 6

[Justin Lebar (not reading bugmail)]

Some SQLs will let you do

SELECT TOP 50 record.fields, findSomewhere(token, record.{title,url}) as result
WHERE result > 0
ORDER BY result, frecency

Comment 7

[Marco Bonardo [:mak]]

The win should be measured using a profile before proceeding with the change.