Fixes #5310. Add ConfigurationManager / Scheme / Theme benchmark baseline#5312
Fixes #5310. Add ConfigurationManager / Scheme / Theme benchmark baseline#5312Copilot wants to merge 4 commits into
Conversation
Adds four BenchmarkDotNet benchmarks under Tests/Benchmarks/Configuration/: - ConfigurationManagerLoadBenchmark: cold-start config load + apply - ThemeSwitchBenchmark: parametric theme switching over all built-in themes - SchemeAttributeBenchmark: GetAttributeForRole at different derivation depths - SchemeSerializationBenchmark: JSON serialize/deserialize round-trip Updates baseline.json with placeholder entries, broadens perf-gate.yml filter to include the new benchmarks, and documents them in README.md. Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/2c601698-792a-447a-889f-2818e993485b Co-authored-by: tig <585482+tig@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/2c601698-792a-447a-889f-2818e993485b Co-authored-by: tig <585482+tig@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c099fb31f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .mcp.json | ||
| .env | ||
| tmpclaude-*-cwd | ||
| local_packages/ |
There was a problem hiding this comment.
Adding local_packages/ to .gitignore while deleting the committed .nupkg/.snupkg files makes fresh checkouts fail the existing LocalPackagesTests (they require that folder plus at least one package file). In the checked workflows (.github/workflows/unit-tests.yml), CI runs dotnet build --no-restore with the default Debug configuration, but CopyNuGetPackagesToLocalPackagesFolder in Terminal.Gui.csproj only runs for Release, so Debug jobs do not recreate local_packages before tests.
Useful? React with 👍 / 👎.
| [IterationSetup] | ||
| public void IterationSetup () | ||
| { | ||
| ConfigurationManager.Disable (true); |
There was a problem hiding this comment.
This benchmark claims to measure cold-start load/apply, but [IterationSetup] runs only once per iteration while BenchmarkDotNet executes the benchmark method many times per iteration. After the first invocation, ConfigurationManager.Enable(...) short-circuits because it is already enabled, so most samples measure a warm Apply() path instead of the documented cold-start path, skewing the baseline used by the perf gate.
Useful? React with 👍 / 👎.
| [IterationSetup] | ||
| public void IterationSetup () | ||
| { | ||
| ThemeManager.Theme = ThemeManager.DEFAULT_THEME_NAME; | ||
| ConfigurationManager.Apply (); | ||
| } |
There was a problem hiding this comment.
Using [IterationSetup] here means the theme is reset to Default only once per iteration, but SwitchTheme() is invoked repeatedly inside that iteration. As a result, only the first call performs an actual switch; subsequent calls mostly reapply the same theme value, so the benchmark under-represents real theme-switch cost and can produce misleading perf-gate baselines.
Useful? React with 👍 / 👎.
|
@copilot ci cd is failing. |
Reverts the accidental removal of local_packages/ directory and the .gitignore addition that caused LocalPackagesTests to fail. Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/8b9c42d7-30b6-4dc2-96ed-15c5f157582c Co-authored-by: tig <585482+tig@users.noreply.github.com>
The CI failures were caused by my second commit accidentally removing the |
Tests/Benchmarks/Configuration/ConfigurationManagerLoadBenchmark.csTests/Benchmarks/Configuration/ThemeSwitchBenchmark.csTests/Benchmarks/Configuration/SchemeAttributeBenchmark.csTests/Benchmarks/Configuration/SchemeSerializationBenchmark.csTests/Benchmarks/baseline.jsonwith placeholder entries for all new benchmark triplesTests/Benchmarks/README.mdto document Configuration benchmarks and update CI references.github/workflows/perf-gate.ymlto include*Config*,*Scheme*,*Theme*filterslocal_packages/and.gitignoreto original state