search: clean up / refactor search result merging code
Created by: slimsag
Prior to this change, the doResults
method suffered from:
- Heavy use of mutexes
- Deeply nested conditionals and large switch statements, making control flow hard to understand.
- Maps being sprinkled in at multiple locations for data deduplication.
- Lock contention: For every symbol result etc. we would re-acquire multiple mutexes.
- Difficult to reason about optional search result logic involving conditionally-acquired
sync.WaitGroup
s.
After this change we:
- Remove the use of mutexes and instead use channels as appropriate.
- Remove deeply nested conditionals and large switch statements in favor of standardization of search functions and using a registration pattern.
- Keep the use of maps for data deduplication close to the source (i.e. when we create the data, not when we read it).
- Solve all lock contention thanks to channels and a clear worker approach.
- No longer need complex conditionally-acquired
sync.WaitGroup
s` because channels work better for waiting on optional search results.
Knowing that others are working on search code heavily, I didn't want to make this change, but in researching adding search pagination I really had no choice: This code was a disaster and initially impossible to reason about.
TL;DR:
- No functionality changes.
- This code is well-tested already.
- This code is now MUCH easier to reason about and read.
Test plan: This code is well-tested already, I am confident this change won't introduce regressions.