Skip to content

improve settings cascade performance

Warren Gifford requested to merge backend-dry-run/cc/cascade-performance into main

Created by: camdencheek

This PR improves the performance of generating merged settings through the settings cascade on sourcegraph.com from ~20ms to ~9ms.

Generating the settings from a cascade of user, org, and site configs is on the critical path of every search request. Currently, it adds ~20ms to the latency of every search request on sourcegraph.com, which is roughly 30% of our backend latency for fast-responding searches. This PR reduces that latency to ~9ms.

Merging settings is surprisingly slow because:

  1. We fetch settings string for each subject, unmarshal them into map[string]interface{}, merge them together, marshal back to a JSON string, then finally unmarshal again back into *schema.Settings
  2. We wait until the settings for every subject is fetched before starting to merge
  3. (last but not least) The JSON representation of the sourcegraph.com site settings is a whooping 244 kilobytes, composed mostly of repogroup definitions. json.Unmarshal() takes about 7 milliseconds for this.

What does this PR do to improve things?

  • It pulls (*settingsCascade).finalTyped(), out of (*settingsCascade).Final() so our search backend can fetch a *schema.Settings without paying the JSON serialization roundtrip cost. Final() still behaves the same by marshalling the result of finalTyped() to a string.
  • It unmarshals the settings for the subject inside the goroutine pool instead of after the pool finishes so unmarshalling is done in parallel
  • Instead of unmarshalling to map[string]interface{}, merging a bunch of interfaces, then marshalling back to a string before ultimately unmarshalling into *schema.Settings it merges *schema.Settings structs directly using reflection. This avoids a full JSON roundtrip, and (IMO), is easier to understand.
  • Now that we use reflection, tests for merging can be more accurate, comprehensive, and descriptive because we can directly test merging *schema.Settings structs more easily.
  • Instead of duplicating the deeply-merged field names as strings which are separated from the types they refer to, merge depth is added as a struct tag.

How can we reduce this even further? Well, currently, ~7ms of the remaining ~9ms is spent on the single unmarshal of the sourcegraph.com global site settings. It takes a long time because of repogroups. With repogroups being deprecated, I expect them to be removed from the site settings and for this to just resolve itself, dropping down to the ~2ms that it now takes to fetch and cascade most normal settings.

P.S. I apologize for the bad diff. git chose some weird spots to hunk.

Merge request reports