Skip to content

Conversation

@JeremyKuhne
Copy link
Member

This is to faciliate writing and running micro benchmarks.

Have to workaround a few issues to make this work:

  1. Can't output to artifacts/bin as it contains a global.json (which breaks project finding logic and other things)
  2. Need to be able to skip _CustomizeXlfSourceNames

Other things:

  • Bump the BenchmarkDotNet version
  • Add the Windows package versions to make it easy to use that functionality when needed

Copy link
Contributor

Copilot AI left a 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 adds infrastructure for writing and running micro-benchmarks using BenchmarkDotNet 0.15.6. The implementation includes a new Benchmark project in the test directory with workarounds for BenchmarkDotNet's project discovery limitations related to global.json files in the artifacts/bin directory.

  • Adds new test/Benchmark project configured as an executable with BenchmarkDotNet integration
  • Updates build infrastructure to support conditional XLF customization and test usings
  • Bumps BenchmarkDotNet package version and adds Windows diagnostics package support

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Benchmark/Program.cs Entry point for running benchmarks with extensive documentation of BenchmarkDotNet integration workarounds
test/Benchmark/InfoTests.cs Sample benchmark demonstrating how to benchmark the dotnet --info command
test/Benchmark/Benchmark.csproj Project configuration setting up executable project with BenchmarkDotNet, excluding test infrastructure
test/Benchmark/Directory.Build.props Custom build settings redirecting output to artifacts/Benchmark and disabling XLF customization
test/Directory.Build.props Makes test global usings conditional via IncludeTestUsings property
build/GenerateResxSource.targets Adds support for skipping XLF source name customization via SkipCustomizeXlfSourceNames property
eng/Versions.props Bumps BenchmarkDotNet to 0.15.6 and adds BenchmarkDotNet.Diagnostics.Windows at 0.14.0
Directory.Packages.props Adds BenchmarkDotNet package versions maintaining alphabetical ordering
sdk.slnx Includes new Benchmark project in solution

This is to faciliate writing and running micro benchmarks.

Have to workaround a few issues to make this work:

1. Can't output to `artifacts/bin` as it contains a global.json (which breaks project finding logic and other things)
2. Need to be able to skip _CustomizeXlfSourceNames

Other things:

- Bump the BenchmarkDotNet version
- Add the Windows package versions to make it easy to use that functionality when needed
DependsOnTargets="_CustomizeResourceNames"
DependsOnTargets="$(CustomizeXlfSourceNamesDependsOn)"
BeforeTargets="PrepareResourceNames;GatherXlf"
Condition="'$(SkipCustomizeXlfSourceNames)'!='true'"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need both this Condition and the change to the DependsOnTargets - if a Condition evaluates to false then the DependsOnTargets should not be run at all. Were you seeing different?

<MicrosoftDotNetInstallerWindowsSecurityTestDataPackageVersion>8.0.0-beta.23607.1</MicrosoftDotNetInstallerWindowsSecurityTestDataPackageVersion>
<BenchmarkDotNetPackageVersion>0.14.0</BenchmarkDotNetPackageVersion>
<BenchmarkDotNetPackageVersion>0.15.6</BenchmarkDotNetPackageVersion>
<BenchmarkDotNetDiagnosticsWindowsPackageVersion>0.14.0</BenchmarkDotNetDiagnosticsWindowsPackageVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<BenchmarkDotNetDiagnosticsWindowsPackageVersion>0.14.0</BenchmarkDotNetDiagnosticsWindowsPackageVersion>
<BenchmarkDotNetDiagnosticsWindowsPackageVersion>0.15.6</BenchmarkDotNetDiagnosticsWindowsPackageVersion>

If these packages version together we may want to use a single property to ensure they don't drift.

Comment on lines +7 to +8
<BaseOutputPath>$([MSBuild]::NormalizeDirectory('$(MSBuildThisFileDirectory)', '..', '..', 'artifacts', '$(MSBuildProjectName)', 'bin'))</BaseOutputPath>
<BaseIntermediateOutputPath>$([MSBuild]::NormalizeDirectory('$(MSBuildThisFileDirectory)', '..', '..', 'artifacts', '$(MSBuildProjectName)', 'obj'))</BaseIntermediateOutputPath>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's an ambient property called ArtifactsRoot or similar that is in use in the repo that gives you the path to the artifacts dir - within that dir, maybe consider using a benchmarks subdirectory for the project-specific output paths to prevent cluttering the 'root' of the artifacts with project-specific folders.

<!-- Global usings -->
<!-- See: https://learn.microsoft.com/dotnet/core/project-sdk/msbuild-props#using -->
<ItemGroup>
<ItemGroup Condition="'$(IncludeTestUsings)'!='false'">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth having a top-level benchmarks directory in the repo root instead of putting it under test - our test structures pull in so much extra stuff that I would expect it to be hard to reason about, and having a separate folder lets your Dir.Build.props/targets anything specific to benchmarks alone really easily.

Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes that may make maintenance/use simpler, but otherwise looks straightforward to me!

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.

3 participants