matchtree: special case word search

[keegancsmith]

A common search Zoekt gets from Sourcegraph is "\bLITERAL\b". With this PR we avoid the regex engine for these type of queries and provide something faster.

Local benchmarks show that the new code runs 4.8x faster for select queries.

Co-authored-by: @stefanhengl

[keegancsmith]

@stefanhengl I couldn't help myself and wrote a little more code which should make this not break anything.

[keegancsmith]

@stefanhengl Thinking about next steps here. I believe this is working. What isn't known if this actually makes a difference.

• Do a simple test with something like hyperfine on a decent corpus
• Eyeball pprof
• Improve implementation. Maybe we should make a wrapper around regex so this is easier to use in things like searcher/etc? Should we somehow implement this optimization in upstream regex?
• Is it worth shipping a custom zoekt to the customer that inspired this optimization?

[stefanhengl]

I ran a couple of tests and it looks like we are finding too many results. EG a search for \bFind\b case:yes will return FindID while the main branch doesn't. I will look into it.

EDIT:
We were treating aA or Aa as word boundary which is not what \bA\b does, hence the many results.

[stefanhengl]

Performance looks promissing (4.8x faster).

→  hyperfine -w 1 "./zoekt_main --index_dir ~/corpus/index \"regex:\bFind\b case:yes\"" "./zoekt_word --index_
dir ~/corpus/index \"regex:\bFind\b case:yes\""
Benchmark 1: ./zoekt_main --index_dir ~/corpus/index "regex:\bFind\b case:yes"
  Time (mean ± σ):     532.7 ms ±   9.0 ms    [User: 1629.3 ms, System: 145.6 ms]
  Range (min … max):   516.2 ms … 548.9 ms    10 runs
 
Benchmark 2: ./zoekt_word --index_dir ~/corpus/index "regex:\bFind\b case:yes"
  Time (mean ± σ):     109.8 ms ±   5.9 ms    [User: 344.9 ms, System: 146.6 ms]
  Range (min … max):   103.3 ms … 127.4 ms    26 runs
 
Summary
  './zoekt_word --index_dir ~/corpus/index "regex:\bFind\b case:yes"' ran
    4.85 ± 0.28 times faster than './zoekt_main --index_dir ~/corpus/index "regex:\bFind\b case:yes"'

[keegancsmith]

\o/

[stefanhengl]

@keegancsmith Shall we roll this out? I think this can be live while we polish it.

[keegancsmith]

@keegancsmith Shall we roll this out? I think this can be live while we polish it.

I think writing a test is a minumum before landing. You wanna do that? Also I wonder if we should record running this code path as another Stat (instead of regexpsconsidered). But regexpsconsidered is probably fine.

Edit: I do believe this change is safe though, so making it in before branch cut would be awesome.

[stefanhengl]

I think writing a test is a minumum before landing. You wanna do that?

👍

[stefanhengl]

Also I wonder if we should record running this code path as another Stat (instead of regexpsconsidered). But regexpsconsidered is probably fine.

Not sure. Given the docstring we probably shouldn't count evaluations of the wordMatchTree as regexp.

// Number of times regexp was called on files that we evaluated.
RegexpsConsidered int

I removed the counter for wordMatchTree. Looking at stats I am not sure counting evaluations of wordMatchTree separately makes sense. The granularity seems too fine compared to the other stats. Additionally we already skip the regexp engine in other places (see regexpToMatchTreeRecursive) and we don't count that either. WDYT @keegancsmith ?

[keegancsmith]

@stefanhengl I can't approve, but LGTM. Ship it :)