Skip to content

search: clean up / refactor search result merging code

Warren Gifford requested to merge sg/general-search-improvements into master

Created by: slimsag

Prior to this change, the doResults method suffered from:

  1. Heavy use of mutexes
  2. Deeply nested conditionals and large switch statements, making control flow hard to understand.
  3. Maps being sprinkled in at multiple locations for data deduplication.
  4. Lock contention: For every symbol result etc. we would re-acquire multiple mutexes.
  5. Difficult to reason about optional search result logic involving conditionally-acquired sync.WaitGroups.

After this change we:

  1. Remove the use of mutexes and instead use channels as appropriate.
  2. Remove deeply nested conditionals and large switch statements in favor of standardization of search functions and using a registration pattern.
  3. Keep the use of maps for data deduplication close to the source (i.e. when we create the data, not when we read it).
  4. Solve all lock contention thanks to channels and a clear worker approach.
  5. No longer need complex conditionally-acquired sync.WaitGroups` 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.

Merge request reports

Loading