feat: add BenchmarkDotNet performance benchmarks#17
Conversation
Closes #12 - Add CronCraft.Benchmarks/ console project targeting net8.0 with BenchmarkDotNet 0.14.0 - 9 benchmark scenarios: simple, day-list, day-of-month, Quartz 6-part, Quartz 7-part, timezone, Spanish, French, and combined day-of-month+weekday - [MemoryDiagnoser] tracks allocations alongside timing - [ShortRunJob] keeps local runs fast; remove for full CI-grade precision - Project added to CronCraft.sln - README updated with run instructions and sample results table Run with: cd CronCraft.Benchmarks && dotnet run -c Release -- --filter * Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new ChangesBenchmarkDotNet Benchmarks Project
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new BenchmarkDotNet-based benchmarks console project to the CronCraft solution to measure ToHumanReadable performance across multiple cron expression variants and locales, and documents how to run those benchmarks.
Changes:
- Introduces
CronCraft.Benchmarksproject with 9 benchmark scenarios and BenchmarkDotNet configuration attributes. - Registers the new benchmarks project in
CronCraft.sln. - Updates
README.mdwith benchmark run instructions and sample output.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Adds benchmark documentation (run instructions + sample results). |
| CronCraft.sln | Adds the benchmarks project to the solution (and new solution platform configs). |
| CronCraft.Benchmarks/CronHelperBenchmarks.cs | Adds the BenchmarkDotNet entrypoint and benchmark scenarios. |
| CronCraft.Benchmarks/CronCraft.Benchmarks.csproj | Adds the new benchmarks console project and BenchmarkDotNet package reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using CronCraft.Extensions; | ||
| using CronCraft.Models; | ||
|
|
||
| BenchmarkRunner.Run<CronHelperBenchmarks>(); |
| { | ||
| _settings = new CronSettings { Language = "en", DayNameFormat = "short" }; | ||
|
|
||
| _timeZone = TimeZoneInfo.FindSystemTimeZoneById("W. Central Africa Standard Time"); |
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>net8.0</TargetFramework> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| <AllowUnsafeBlocks>true</AllowUnsafeBlocks> | ||
| </PropertyGroup> |
| GlobalSection(SolutionConfigurationPlatforms) = preSolution | ||
| Debug|Any CPU = Debug|Any CPU | ||
| Debug|x64 = Debug|x64 | ||
| Debug|x86 = Debug|x86 | ||
| Release|Any CPU = Release|Any CPU | ||
| Release|x64 = Release|x64 | ||
| Release|x86 = Release|x86 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CronCraft.Benchmarks/CronHelperBenchmarks.cs`:
- Line 23: The benchmark setup uses TimeZoneInfo.FindSystemTimeZoneById with a
Windows-specific timezone ID that may fail on non-Windows systems without ICU
support, causing TimeZoneNotFoundException and breaking the benchmark on
contributor machines. Create a helper method (e.g., TryResolveTimeZone) that
attempts to resolve the timezone ID but includes try-catch error handling for
TimeZoneNotFoundException and InvalidTimeZoneException, with a fallback to
TimeZoneInfo.Utc if resolution fails. Then replace the direct
FindSystemTimeZoneById call with a call to this new helper method to ensure the
benchmark setup succeeds across all environments.
In `@README.md`:
- Around line 114-115: The unquoted wildcard `*` in the dotnet filter commands
on lines 114 and 127 will be expanded by the shell before dotnet receives them,
causing unexpected filtering behavior. Quote the wildcard filter argument in
both locations to prevent shell glob expansion: change `--filter *` to `--filter
"*"` on line 114 and similarly on line 127 so the literal asterisk is passed to
dotnet as intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a3f7486-f5ee-42ce-a6ec-8fad689e1cbd
📒 Files selected for processing (4)
CronCraft.Benchmarks/CronCraft.Benchmarks.csprojCronCraft.Benchmarks/CronHelperBenchmarks.csCronCraft.slnREADME.md
| { | ||
| _settings = new CronSettings { Language = "en", DayNameFormat = "short" }; | ||
|
|
||
| _timeZone = TimeZoneInfo.FindSystemTimeZoneById("W. Central Africa Standard Time"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "CronHelperBenchmarks.cs" --exec cat -n {}Repository: teesofttech/CronCraft
Length of output: 2834
🌐 Web query:
.NET TimeZoneInfo FindSystemTimeZoneById Windows timezone ID non-Windows platforms cross-platform IANA
💡 Result:
In.NET 6 and later, the TimeZoneInfo.FindSystemTimeZoneById method provides cross-platform support for both Windows time zone IDs (e.g., "Pacific Standard Time") and IANA time zone IDs (e.g., "America/Los_Angeles") regardless of the underlying operating system [1][2][3]. Key details regarding this functionality include: Integration and Requirements The runtime achieves this by leveraging the ICU (International Components for Unicode) library [1][4]. For this cross-platform resolution to function, the environment must have ICU support enabled [4]. On Windows, this requires that the application not be running in globalization invariant mode and that NLS (National Language Support) is not enabled [1][4]. On Linux-based environments (such as Docker containers), ensure the necessary time zone data (often provided by the tzdata package) is installed, as some minimal images may not include it by default [5]. Behavior When you call FindSystemTimeZoneById with an ID, the method attempts to resolve the identifier using the available system provider [1][3]. If the runtime detects an ID that does not match the native format of the host OS, it can automatically perform the necessary conversion to resolve the time zone correctly [5][3]. This eliminates the need for manual mapping or third-party conversion libraries in many modern applications [6][7]. Compatibility Note While.NET 6+ simplified this process, older versions of.NET (such as.NET Framework or early.NET Core versions) did not support cross-platform ID resolution [8][9]. In those environments, developers were required to either manually handle the differences using conditional logic or use community libraries like TimeZoneConverter to map between Windows and IANA identifiers [5][6][10]. For applications targeting.NET 6+, you can generally use either ID format with confidence, provided your environment is correctly configured with ICU support [1][4]. If you encounter issues in specific deployments, verifying that the host system has the required ICU and timezone data installed is the recommended first step [4][5].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.timezoneinfo.findsystemtimezonebyid?view=net-9.0
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.timezoneinfo.findsystemtimezonebyid?view=net-10.0
- 3: https://codeblog.jonskeet.uk/2022/02/05/whats-up-with-timezoneinfo-on-net-6-part-1/
- 4: https://learn.microsoft.com/en-us/dotnet/api/system.timezoneinfo.tryconvertwindowsidtoianaid?view=net-8.0
- 5: https://stackoverflow.com/questions/41566395/timezoneinfo-in-net-core-when-hosting-on-unix-nginx
- 6: https://github.com/mattjohnsonpint/TimeZoneConverter
- 7: https://world.optimizely.com/blogs/sheider-devnotes/dates/2024/4/-net-core-timezone-ids-windows-vs-linux/
- 8: https://devblogs.microsoft.com/dotnet/cross-platform-time-zones-with-net-core/
- 9: TimeZoneInfo should have consistent Ids across Windows and Linux dotnet/runtime#14929
- 10: TimeZoneInfo.FindSystemTimeZoneById throws TimeZoneNotFoundException dotnet/runtime#16917
🏁 Script executed:
find . -name "*.csproj" -o -name "*.fsproj" | grep -i benchmark | head -5 | xargs catRepository: teesofttech/CronCraft
Length of output: 545
Add error handling for timezone resolution to ensure benchmark portability.
Line 23 uses a Windows timezone ID that requires proper environment configuration (ICU support and timezone data) to work cross-platform with .NET 8. While .NET 8 can resolve Windows IDs on non-Windows systems, environments without ICU or timezone data will fail with TimeZoneNotFoundException. Add a try-catch fallback or attempt alternative IDs to ensure the benchmark setup succeeds across all contributor environments.
Suggested patch
- _timeZone = TimeZoneInfo.FindSystemTimeZoneById("W. Central Africa Standard Time");
+ _timeZone = TryResolveTimeZone("W. Central Africa Standard Time", "Africa/Lagos");private static TimeZoneInfo TryResolveTimeZone(params string[] ids)
{
foreach (var id in ids)
{
try { return TimeZoneInfo.FindSystemTimeZoneById(id); }
catch (TimeZoneNotFoundException) { }
catch (InvalidTimeZoneException) { }
}
return TimeZoneInfo.Utc;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CronCraft.Benchmarks/CronHelperBenchmarks.cs` at line 23, The benchmark setup
uses TimeZoneInfo.FindSystemTimeZoneById with a Windows-specific timezone ID
that may fail on non-Windows systems without ICU support, causing
TimeZoneNotFoundException and breaking the benchmark on contributor machines.
Create a helper method (e.g., TryResolveTimeZone) that attempts to resolve the
timezone ID but includes try-catch error handling for TimeZoneNotFoundException
and InvalidTimeZoneException, with a fallback to TimeZoneInfo.Utc if resolution
fails. Then replace the direct FindSystemTimeZoneById call with a call to this
new helper method to ensure the benchmark setup succeeds across all
environments.
| dotnet run -c Release -- --filter * | ||
| ``` |
There was a problem hiding this comment.
Quote benchmark filter wildcards to avoid shell glob expansion.
The commands on Line 114 and Line 127 use unquoted *; shells may expand these before dotnet receives them, causing unexpected filtering behavior.
Suggested patch
-dotnet run -c Release -- --filter *
+dotnet run -c Release -- --filter "*"
...
-> Tip: add `--filter *Simple*` to run a single benchmark by name.
+> Tip: add `--filter "*Simple*"` to run a single benchmark by name.Also applies to: 127-127
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 114 - 115, The unquoted wildcard `*` in the dotnet
filter commands on lines 114 and 127 will be expanded by the shell before dotnet
receives them, causing unexpected filtering behavior. Quote the wildcard filter
argument in both locations to prevent shell glob expansion: change `--filter *`
to `--filter "*"` on line 114 and similarly on line 127 so the literal asterisk
is passed to dotnet as intended.
|
🎉 Thank you for your contribution! This PR was included in CronCraft v1.0.6 and is now live on NuGet for everyone to use. We really appreciate the time and effort you put in — it makes the library better for the whole community! 🚀 |
Closes #12
Summary
CronCraft.Benchmarks/as a new console project using BenchmarkDotNet 0.14.0CronCraft.slnBenchmark scenarios (9 total)
*/5 * * * *0 */2 * * 1,2,3,4,50 4 27 */6 ?0 15 10 * * ?0 0 12 1/1 * ? *30 14 * * *+ WCAST0 9 * * 1(es)0 9 * * 1(fr)0 23 15 * 1[MemoryDiagnoser]is enabled so allocation counts appear alongside timing.[ShortRunJob]keeps local runs fast — remove it for full statistical precision.How to run
Test plan
dotnet build CronCraft.sln --configuration Release— succeeds, 0 warningsSummary by CodeRabbit
New Features
Documentation