Fixes #5310. Add ConfigurationManager / Scheme / Theme benchmark baseline#5312
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>
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".
|
@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 |
|
@copilot addrss CR Feedback |
Move ConfigurationManager.Disable(true) into the LoadAndApply benchmark method so every invocation is a true cold start (Enable short-circuits when already enabled). Move theme reset into SwitchTheme so every invocation performs a real theme change instead of redundantly reapplying. Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/26cdfc29-ec96-446a-b9fb-da0dcadc6136 Co-authored-by: tig <585482+tig@users.noreply.github.com>
…packages Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/26cdfc29-ec96-446a-b9fb-da0dcadc6136 Co-authored-by: tig <585482+tig@users.noreply.github.com>
Addressed both CR items in 7d1e9f6:
Also removed accidentally committed 1.0.0 nupkg build artifacts in f0f2b54. |
|
@copilot why does this workflow get skipped? https://github.com/gui-cs/Terminal.Gui/actions/runs/25974725214/job/76353177797?pr=5312 |
The "Benchmarks (Linux, ShortRun)" job is skipped by design — line 69 of If you'd like benchmarks to also run on PRs, I can remove that condition. |
@copilot I don't want to change it tor un on PRs in general, but I need confidence that for THIS Pr it runs fine. We need to actually generate the baseline with this PR. |
Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/8f0e14fd-f941-4c23-b417-91490e8bf050 Co-authored-by: tig <585482+tig@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/8f0e14fd-f941-4c23-b417-91490e8bf050 Co-authored-by: tig <585482+tig@users.noreply.github.com>
Ran all 15 Configuration benchmarks locally on the same GitHub Actions runner hardware (AMD EPYC 7763, .NET 10.0.5) and updated
The benchmarks job in CI will still be skipped on PRs (by design), but when this merges to |
There was a problem hiding this comment.
Pull request overview
Adds a BenchmarkDotNet baseline for the ConfigurationManager/Theme/Scheme subsystems so the upcoming VisualRole.Code* expansion (#5311) can be measured against a fixed reference. The perf-gate workflow and baseline data are widened to include these new benchmarks alongside the existing Scrolling suite.
Changes:
- New benchmarks under
Tests/Benchmarks/Configuration/for cold-start config load, theme switching (parametric over all built-in themes), per-role attribute lookup at varying derivation depths, andSchemeJSON serialize/deserialize/round-trip. baseline.jsonextended with measured means captured on ubuntu-latest (EPYC 7763, .NET 10.0.5).perf-gate.ymlfilter and labels broadened from "Scrolling" to general "Performance Gate"; README documents the new benchmarks and updated commands.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/Benchmarks/Configuration/ConfigurationManagerLoadBenchmark.cs | Cold-start Disable→Enable→Apply benchmark for embedded library config. |
| Tests/Benchmarks/Configuration/ThemeSwitchBenchmark.cs | Parametric theme-switch benchmark over all built-in theme names. |
| Tests/Benchmarks/Configuration/SchemeAttributeBenchmark.cs | Measures GetAttributeForRole at three derivation depths. |
| Tests/Benchmarks/Configuration/SchemeSerializationBenchmark.cs | Serialize/Deserialize/RoundTrip for a representative Scheme. |
| Tests/Benchmarks/baseline.json | Adds measured baselines for the new benchmarks; updates header comments. |
| Tests/Benchmarks/README.md | Documents the new Configuration benchmarks and updated run commands. |
| .github/workflows/perf-gate.yml | Broadens job name, summary title, and --filter to include Config/Scheme/Theme. |
…-separated Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/2dcbd1e9-8ca7-4562-ae6b-5fb1472cf0d2 Co-authored-by: tig <585482+tig@users.noreply.github.com>
Fixes
Adds benchmark baseline for the configuration/theming layer so the follow-up PR adding 12
VisualRole.Code*entries (#5311) can demonstrate no regression mechanically.Proposed Changes/Todos
Four BenchmarkDotNet benchmarks under
Tests/Benchmarks/Configuration/:ConfigurationManagerLoadBenchmark—Disable(true)→Enable(LibraryResources)→Apply()cold-start path. Reset is performed inside each invocation so every sample is a true cold start (Enableshort-circuits when already enabled).ThemeSwitchBenchmark—ThemeManager.Theme = "X"; Apply()parametric over all built-in themes via[ParamsSource]. Resets toDefaultinside each invocation so every sample performs a real theme change.SchemeAttributeBenchmark—GetAttributeForRole()at three derivation depths:Normal(explicit),HotFocus(1-hop),Code(deepest chain)SchemeSerializationBenchmark—JsonSerializer+SchemeJsonConverterserialize/deserialize/round-tripbaseline.json— Real baseline numbers from benchmark run on GitHub Actions ubuntu-latest (AMD EPYC 7763, .NET 10.0.5), including all 8 built-in themes (added missing "8-Bit" theme entry)perf-gate.yml— broadened filter to'*Scroll*' '*Config*' '*Scheme*' '*Theme*'; updated job names and summary titleREADME.md— documented Configuration benchmarks, updated CI and baseline-update references; fixed--filtersyntax to use space-separated globs (not pipe-separated)Baseline Results
Pull Request checklist:
CTRL-K-Dto automatically reformat your files before committing.dotnet testbefore commit///style comments)