Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic config conversion improvements #7052

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Conversation

dnr
Copy link
Member

@dnr dnr commented Jan 7, 2025

** Do not merge until we're using Go 1.24 **

What changed?

  • Split implementation of "constrained default" settings from "plain default" settings. This is more code and the diff looks complex, but the individual paths are both simpler than the mixed version.
  • Add conversion cache using a weak map (requires Go 1.24).
  • Remove GlobalCachedTypedValue.
  • Use "raw" values for subscription dispatch deduping to avoid unnecessary conversions there.
  • Deep copy default values when using mapstructure, to avoid problems with merging over shared default values.

Why?

  • Fixes Optimize dynamic config value conversion #6756
  • Performance improvement for "plain default" settings (almost all of them)
  • Performance improvement for settings with complex converters
  • Remove footgun in defaults that aren't scalar values

How did you test it?

existing+new unit tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize dynamic config value conversion
1 participant