-
Notifications
You must be signed in to change notification settings - Fork 12
perf: cache prefixed keys on TemplateResource initialization #457
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
Conversation
Cache the result of util.AppendPrefix() on TemplateResource initialization to avoid repeated computation on every process() cycle. Changes: - Add prefixedKeys field to TemplateResource struct - Compute prefixedKeys once in NewTemplateResource after normalizing prefix - Update backendFetcher to store and use pre-computed prefixedKeys - Change fetchValues() signature to use stored keys instead of parameter - Update processor.go and preflight.go to use cached prefixedKeys Fixes #445
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #457 +/- ##
==========================================
- Coverage 59.82% 59.80% -0.02%
==========================================
Files 48 48
Lines 5244 5244
==========================================
- Hits 3137 3136 -1
- Misses 1901 1902 +1
Partials 206 206 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR optimizes template processing by caching prefixed keys at initialization time rather than computing them repeatedly during each process cycle. This addresses performance issue #445 by eliminating redundant util.AppendPrefix() calls.
Changes:
- Add
prefixedKeysfield toTemplateResourcestruct, computed once during initialization - Update
backendFetcherto store and use pre-computed prefixed keys - Remove
keysparameter fromfetchValues()method signature
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/template/resource.go | Add prefixedKeys field and compute it during initialization; pass to backend fetcher |
| pkg/template/processor.go | Replace repeated util.AppendPrefix() calls with cached prefixedKeys field |
| pkg/template/preflight.go | Use cached prefixedKeys instead of computing on-demand |
| pkg/template/backend_fetcher.go | Store prefixed keys and remove parameter from fetchValues() |
| pkg/template/backend_fetcher_test.go | Update test cases to provide PrefixedKeys in config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Gid int | ||
| Group string | ||
| Keys []string | ||
| prefixedKeys []string // Cached keys with prefix applied |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field prefixedKeys should follow Go naming conventions and be exported as PrefixedKeys since it's being passed to other packages (e.g., in backendFetcherConfig). Alternatively, if it should remain unexported, the backendFetcherConfig.PrefixedKeys field should also be unexported.
| storeClient backends.StoreClient | ||
| store *memkv.Store | ||
| prefix string | ||
| prefixedKeys []string // Pre-computed keys with prefix applied |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field prefixedKeys is unexported but receives data from an exported field PrefixedKeys in the config struct. For consistency, either both should be exported or both should be unexported.
Summary
util.AppendPrefix()onTemplateResourceinitialization to avoid repeated computation on everyprocess()cycleprefixedKeysfield toTemplateResourcestruct computed once during initializationbackendFetcherto store and use pre-computed prefixed keysfetchValues()signature to use stored keys instead of parameterTest plan
go test ./...)make build)Fixes #445
🤖 Generated with Claude Code