Fix the csproj build plumbing's output paths and reference gathering - #16
Merged
Merged
Conversation
* allow affinity mask wider than 32 processors the affinity cli option was parsed as int so a mask with any bit above 32 could not be passed at all even though the job stores it as IntPtr and FixAffinity already handles a 64 bit mask parsing it as long also means the perfonar model must hold long or the value gets truncated on the way out * do not run the wide mask test on a 32 bit runtime new IntPtr(1L << 40) throws OverflowException where IntPtr is four bytes, so the test i added would fail on a 32 bit run rather than prove anything. added a Platform64BitOnly requirement so it skips there, using the same FactEnvSpecific mechanism the other platform bound tests already use also added a test for the top bit. the 64th cpu is the last one FixAffinity handles without cpu groups and its mask only fits in a signed long as the negative value, so this pins down that it still reaches the right IntPtr * make the affinity option unsigned a mask is not a signed number and the 64th cpu needed to be written as -9223372036854775808 to reach it. now it is written the way the mask reads the perfonar model stays long. perfolizers LightJsonSerializer throws Unsupported type: System.UInt64 so making that half unsigned breaks PerfonarTableTest at runtime. the value is only carried there so a signed long holds the same bits and nothing is lost conversion to IntPtr goes through new IntPtr(unchecked((long)value)) which is the same shape FixAffinity already uses * reject an affinity mask a 32 bit process can not hold the option is now unsigned so a value above int.MaxValue can reach new IntPtr(long) which throws on a 32 bit process instead of truncating. that made ConfigParser.Parse fail with an unhandled OverflowException rather than a normal option error. the conversion now goes through TryConvertAffinity which takes the pointer size. on 8 bytes it keeps the full 64 bit mask as before. on 4 bytes it takes the low 32 bits so a full 32 processor mask still works and it reports failure for anything wider so Validate can print an error.
* Refactor toolchains and runtimes * Fixes from AI review * Restore master changes an earlier rebase dropped.
…ssemblies (#3250) The weaver task's dependencies are packed into tasks/netstandard2.0 of both BenchmarkDotNet.Weaver and BenchmarkDotNet.Annotations, so AsmResolver's binaries are redistributed to every consumer with no dependency edge and no MIT notice. Add THIRD-PARTY-NOTICES.txt next to the weaver and pack it at the root of the Annotations package, which is the one that ships standalone. CopyLocalLockFileAssemblies also copied five assemblies the task never needs: Microsoft.NET.StringTools, System.Buffers, System.Memory, System.Numerics.Vectors and System.Runtime.CompilerServices.Unsafe. All are transitive from the MSBuild packages and supplied by the MSBuild host at runtime; AsmResolver's netstandard2.0 build references only netstandard. ExcludeAssets="runtime" on the two MSBuild references drops them, leaving the four AsmResolver assemblies. Also update AsmResolver 6.0.0 -> 6.0.1. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Shorten NativeAOT and ReadyToRun build paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove workaround-specific tests --------- Co-authored-by: Shubhra Mittal <shmitt@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add async enumerable support to `ParamsSource`/`ArgumentsSource`. Refactored parameter discovery and codegen. Added/removed analyzer rules. Added tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Adjust codegen to avoid reserved member names altogether. * Using static RunnableConstants to improve readability. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Add job categories Job categories are the job equivalent of [BenchmarkCategory]: they allow grouping jobs so that a subset of them can be selected, without relying on the job Id. - MetaMode.Categories, a hidden characteristic so that categories don't affect the generated job Id, the folder names, the summary nor the code generated for the child process - Job.WithCategory (adds) and Job.WithCategories (overrides), mirroring WithEnvironmentVariable/WithEnvironmentVariables - [JobCategory] attribute, which adds its categories to every job defined for the given class or assembly. It's implemented as a mutator job, so ImmutableConfigBuilder now merges categories instead of overriding them - JobCategoryFilter, which selects the benchmarks of the jobs that belong to any of the given categories * Update job category assignment in benchmark examples Replaced [JobCategory] with the Categories property in SimpleJob attributes, allowing multiple categories to be set inline. Updated documentation to clarify that categories do not affect job execution details. * Remove JobCategoryAttribute and related logic Deleted the JobCategoryAttribute.cs file, including all using directives, class definition, constructors, and logic for applying job categories to jobs via attributes. This removes support for assigning categories to jobs through this attribute. * Refactor JobConfigBaseAttribute for lazy config & categories Refactor JobConfigBaseAttribute to use lazy config initialization and support job categories. Add a Categories property for job filtering and update constructors to accommodate these changes. Improve comments and code clarity. * Remove category preservation when applying mutator jobs Previously, job categories were explicitly preserved and re-added after applying mutator jobs to ensure they were not lost. This logic has been removed, and mutators are now applied directly without restoring categories. * Refactor and expand job category tests Replaced WithTwoJobsAndACategory with WithTwoCategorizedJobs using explicit Categories in SimpleJob attributes. Added tests for category assignment, jobs without categories, category-independent job IDs, and category-based filtering. Introduced new test classes and removed obsolete tests and attributes. * Make Categories init-only and nullable in JobConfigbaseAttribute Refactored the Categories property to be init-only and nullable (string[]?), enforcing immutability after initialization. Updated Config property logic to handle null or empty Categories safely. Added remarks to clarify attribute usage and rationale for these changes. * Improve job selection to merge categories correctly Updated job selection logic to group jobs by equality (excluding categories) and merge their categories, replacing Distinct with JobComparer. Added MergeCategories method to ensure all categories are preserved, preventing mismatches when filtering jobs by category. * Skip category in job deduplication comparison The comparison logic for jobs now ignores the MetaMode.CategoriesCharacteristic field. This change prevents jobs that differ only by category from being treated as distinct, ensuring the same job isn't run multiple times under different names. * Add null check for categories in WithCategories method Updated the WithCategories extension for Job to throw ArgumentNullException if categories is null. This improves error handling and prevents potential runtime exceptions. * Improve Categories setter to handle empty/null values Updated the Categories property setter to set the value to null if the input is null or results in an empty set after removing duplicates. This change ensures jobs with no categories are treated the same as those never assigned categories, preventing unnecessary updates during category selection. * Improve job category tests: deduplication, null, ordering Expanded and clarified the BenchmarkDotNet job category test suite. Renamed and rewrote the deduplication test to verify merging of categories. Added tests for ordering (ignoring categories), handling empty and null categories, and correct exception parameter naming. Introduced a new test class for null category scenarios to ensure robust and correct category handling. * Clarify job category filtering in documentation Updated docs to explain three methods for filtering jobs by category: JobCategoryFilter in code, [JobCategoryFilter] attribute, and --jobCategories argument. Added code and CLI examples. Clarified that filters are inclusion-only and jobs without categories are excluded unless assigned. * Document --jobCategories option in BenchmarkDotNet CLI Updated documentation to describe the new --jobCategories console argument. This option enables running benchmarks for jobs in specified categories, excluding uncategorized jobs. Changes include updates to the usage list and detailed options section. * Add JobCategoryFilterAttribute for benchmark filtering Introduced JobCategoryFilterAttribute in BenchmarkDotNet.Attributes to enable filtering benchmarks by job categories. Includes constructors for CLS compliance and category specification, XML documentation, and PublicAPI annotation. * Preserve job categories when applying mutator jobs Previously, mutating jobs could overwrite existing categories, leading to loss of category information. Now, the original categories are saved, the mutator is applied, and both sets of categories are combined to ensure correct job selection by category. * Add JobCategories filter to CommandLineOptions Added JobCategories property to CommandLineOptions for filtering benchmarks by job category. Updated UserProvidedFilters logic to recognize JobCategories as a user-provided filter. * Add job category filtering to benchmark config parser Added support for filtering benchmarks by job categories in the configuration parser. If job categories are specified in the options, a JobCategoryFilter is created and applied, enabling selective benchmark execution. * Clarify JobCategoryFilter remarks in XML docs Added a detailed <remarks> section to the JobCategoryFilter class summary. The new remarks clarify that the filter is inclusion-only and explain how benchmarks with uncategorized jobs are handled when filtering by category. No code logic was changed. * Refactor WithCategories to expression-bodied method Refactored the WithCategories method in BenchmarkDotNet.Jobs to a single-line expression-bodied method, removing the explicit ArgumentNullException check for the categories parameter. * Enhance validation for job categories in MetaMode Improved validation and error handling in BenchmarkDotNet.Jobs.MetaMode. AddCategories now throws ArgumentNullException for null input. Unique method checks for null input and null categories, throwing exceptions as needed. Updated comments to clarify validation and merging logic. * Enhance JobCategoryTests with comprehensive scenarios Added extensive tests to JobCategoryTests.cs covering category merging, handling of null/empty categories, and category filtering via attributes and console arguments. Ensured categories are additive, invalid inputs are rejected, and introduced supporting test classes and attributes. * Clarify job attribute categories and their usage Clarified that only attributes deriving from JobConfigBaseAttribute (e.g., [SimpleJob], [DryJob], [ShortRunJob], [InProcess]) have a Categories property. Noted that mutator attributes (like [WarmupCount], [IterationCount], etc.) do not have Categories and require the fluent API for category assignment. Also specified that categories do not affect job execution, IDs, folder names, or summaries. * Clarify comments and improve category merging logic Comments in ImmutableConfigBuilder.cs were clarified, especially regarding category handling for mutators. The MergeCategories method now deduplicates categories case-insensitively before merging, preventing unnecessary copies. When merging, the IsMutator flag is explicitly restored to maintain correct mutator behavior. * Test deduplication and clarify job category handling Added tests for deduplicated mutator jobs to verify category retention after merging. Renamed a test for clarity on category checks. Added comments to explain mutator job and category merging behavior during deduplication and job copying. * Merge job categories with the reworked JobComparer JobComparer.Equals no longer delegates to Compare, so the skip that keeps categories out of the comparison had to be repeated there. That is the overload GroupBy and Distinct use, so without it two jobs differing only by category no longer collapse and the benchmark runs twice under one name. The deduplication that now runs after the mutators are applied merges the categories instead of dropping them, for the same reason the first pass does: the comparer ignores categories, so a job collapsed there can carry ones the survivor does not have, and selecting by those would match nothing. * Update docs: add JobCategoryFilter and clarify args Updated filters documentation to include the new JobCategoryFilter, enabling filtering by job category names with --anyJobCategories. Revised filter table, examples, and clarified correct console argument usage. Updated code and command-line examples for job category filtering. * Make Categories property settable for C# 7.3 compatibility Changed Categories in JobConfigbaseAttribute from init-only to settable to support projects targeting net472 and netstandard2.0 (C# 7.3). Updated remarks to clarify compatibility and assignment requirements. * Rename --jobCategories to --anyJobCategories in docs Updated all documentation references from --jobCategories to --anyJobCategories to clarify that the argument runs benchmarks for any of the specified job categories. This improves accuracy and consistency in the documentation. * Rename JobCategories to AnyJobCategories for consistency Standardize job category filter naming by renaming the JobCategories property to AnyJobCategories in CommandLineOptions and updating all related references, including the Option attribute and filter logic in ConfigParser.cs. This aligns with the "Any" prefix convention used elsewhere. * Improve category argument validation and error reporting Refactor category-related methods in BenchmarkDotNet.Jobs to centralize and strengthen argument validation. Introduce SetCategories for consistent null checks and error messages, update WithCategory, WithCategories, MetaMode.Categories, and AddCategories to use it, and ensure deduplication and removal logic is preserved. Error messages now reference correct parameter names for clarity. * Clarify test comments and update exception checks - Improved comments in AnEmptySetOfCategoriesClearsTheCategoriesTheJobAlreadyHad for clarity on guard coverage. - Updated expected exception parameter names to "value" in ANullSetOfCategoriesIsRejectedWhicheverWayItIsPassed and ANullCategoryIsRejected; added explanatory comments. - In ANullCategoryIsRejected, now use new Job instances for AddCategories and assignment checks; added assertion for WithCategory(null) throwing ArgumentNullException with "category" parameter. - Changed command-line argument in TheFilterIsAvailableAsAConsoleArgument from "--jobCategories" to "--anyJobCategories".
* chore: modify powercfg output parse logics * chore: update skip logics based on review comment * chore: add finally block * chore: fix try-finally block code
…s true (#3252) * Skip BenchmarkDotNetWeaveAssemblies target if SkipCompilerExecution is true When SkipCompilerExecution is true, csc task would not run compiler, therefore no output assembly will be built. * Fix condition formatting in BenchmarkDotNet.Common.targets
Add .NET 12 monikers, cached runtimes and toolchains including composite ReadyToRun and Mono WebAssembly. Update development defaults and templates, and cover runtime parsing and CLI resolution. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Output paths: - --no-dependencies was widened to every autogenerated build via a filename check. The generated project keeps its ProjectReference, and RIDs propagate to exe references but not library ones, so the reference resolved to a RID-specific ref assembly that nothing produced. Reverted to the ForcedNoDependenciesForIntegrationTests gate. - GetPublishDirectoryPath defaulted to <artifacts>/publish while the generators expect the executable in their binaries directory. That was masked while --output overrode PublishDir on the command line; once PublishDir became authoritative, publish wrote where BenchmarkDotNet never looks. - OutputPath was derived by chopping TFM.Length + 1 characters off the binaries path, which is wrong wherever that path ends in the RID. The generated project sets AppendTargetFrameworkToOutputPath=false instead. - The generated project sets OutDir as well as OutputPath. OutDir only falls back to OutputPath when it has not been set, so a Directory.Build.props in the benchmark project's tree that sets OutDir kept it, and the build went somewhere BenchmarkDotNet does not look. The command line arguments this replaces set both, which is why the case did not arise before. Reference gathering: The gatherer globbed *.dll out of the benchmark project's output, which holds more than references: content files copied there, and native libraries. A native library handed to MSBuild as an assembly is MSB3246. A content file is published by the ProjectReference already, so referencing it too puts the same file in the publish set twice, and crossgen2 rejects duplicate inputs - the NETSDK1152 that R2R and Wasm were suppressing. And an exe project has no dll beside its exe, so on .NET Framework the benchmark assembly was never gathered at all. Asking MSBuild for @(ReferencePath) avoids all three, so both suppressions go. It also built DllGatherer.csproj, a copy of the generated project made from the default template, which could not match what a toolchain had actually generated. The gathering pass now builds the generated project itself with SkipCompilerExecution, so it stops once MSBuild has resolved the references and needs neither a second project, a stub entry point, nor the output-path handshake between the two. Not running the compiler also keeps the pass from writing a program built against incomplete references into the directory BenchmarkDotNet reads the benchmark executable from. The gathering pass no longer edits the project it just built either. It writes what MSBuild resolved beside the project, and the project imports that file when it exists, so the project is complete the moment it is written. NativeAot passed -r on the command line while its project already declared the RuntimeIdentifier. Only the command line one reaches the referenced projects, so the benchmark project was built once for gathering and again for the real build, into two different pivots, and the references gathered came from the pivot that was not used. Project generation: The generated project is composed as an XDocument rather than substituted into a text template, so a path holding an XML character cannot corrupt it, and a toolchain extends it by overriding AddEarlyProperties, AddProjectContent or AddLateProperties instead of maintaining a near-copy of the whole file. R2R, Wasm and NativeAot each kept their own template that had drifted from the default one; they now share the parts they never meant to differ on, which is also what puts the gathered references in front of all four toolchains rather than only the default one. That leaves nothing importing BenchmarkDotNet.Build.props or .targets, so those and the three csproj templates are gone, along with the GenerateCustomBuildHooksAsync hook added to write them. Build invocations: Publishing ran as three dotnet invocations - restore, then build, then publish - when the publish command performs all three itself, so that step is one invocation now. Restoring separately is still what the gathering pass does, and what a toolchain that only builds does, since those are the passes that have to do the restoring. A custom build configuration takes a single invocation as well, a standalone restore having no way to be told which configuration to use. Building without dependencies now covers the publishing toolchains too. That is only safe because no toolchain passes a runtime identifier on the command line any more, where it is a global property and reaches the referenced projects: a library has no runtime identifier to resolve against, and not building it is what stops the sdk from negotiating one away. Mono was the last to do so and declares SelfContained and the identifier in the project instead, which leaves MonoPublisher with nothing to do and the helper that built NativeAot's argument with no callers. Publishing with --no-build is what raised NETSDK1085 on Wasm, and nothing passes it any more, so that project no longer turns BuildProjectReferences off to suppress it. SDK requirement: ArtifactsPath is set as a property rather than passed as --artifacts-path, which is a .NET 8 SDK argument. An older SDK errors on the argument but ignores the property, so nothing has to require that SDK: master's fallback of building net7 and earlier one partition at a time already covers the isolation those builds then go without. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
|
On my local environment. when running samples benchmarks it raise Weaver relating error. Reproduce Steps
Error Message It raise error though benchmark run continued. |
filzrev
reviewed
Sep 14, 2026
filzrev
reviewed
Sep 14, 2026
| // Set as a property rather than --artifacts-path, which is a .NET 8 SDK argument: an older SDK | ||
| // errors on the argument but simply ignores the property. Those partitions get no isolated | ||
| // intermediate output, which is why BenchmarkRunnerClean builds them one at a time. | ||
| return $"/p:ArtifactsPath={artifactsPath.QuoteIfNeeded()}"; |
Owner
There was a problem hiding this comment.
Personally, I prefer setting the minimum requirement to .NET 8 and using --artifacts-path to specify a relative path, but if backward compatibility is a priority, I suppose there’s no choice.
Author
There was a problem hiding this comment.
The thing is it's cheap to support older sdks by simply not building in parallel. I'm going to have a follow-up change to builders for them to declare that they support concurrency instead of hard-coding it in the runner.
Owner
|
Please ignore any subsequent commits and PR comments. I'll try to setup Pullfrog for automated code review in this PR. |
Author
I think that's because we forgot to update the develop package in dotnet#3252. I pushed that fix. |
Owner
|
@timcassell For reference, I've tested automated PR review by using Pullfrog (with model I'm not sure these suggested changes need to be handled or not though. Pullfrog providing support for OSS projects. |
Author
No changes necessary from there. All 3 items turned out wrong or unreproducible. But the investigation did reveal a regression on master which will be fixed by dotnet#3255.
You can just merge this and your PR will be updated. |
filzrev
merged commit Sep 16, 2026
e3efdf2
into
filzrev:chore-fix-double-write-issue
10 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up fixes on top of dotnet#3235, targeted at
chore-fix-double-write-issueso they land in that PR.The two commits
Merge branch 'master'— the branch had not had master merged since chore: Modify powercfg output parse logics dotnet/BenchmarkDotNet#3238, so this brings in 11 commits (Refactor toolchains and runtimes dotnet/BenchmarkDotNet#3232, Add job categories dotnet/BenchmarkDotNet#3239, allow affinity mask wider than 32 processors dotnet/BenchmarkDotNet#3242, Shorten NativeAOT and ReadyToRun build paths dotnet/BenchmarkDotNet#3244, chore: Modify file cleanup logics for .NET Framework dotnet/BenchmarkDotNet#3247, Support async sources dotnet/BenchmarkDotNet#3248, Update dotMemory/dotTrace TFMs dotnet/BenchmarkDotNet#3249, Add third-party notices for bundled AsmResolver, drop host-provided assemblies dotnet/BenchmarkDotNet#3250, Skip BenchmarkDotNetWeaveAssemblies target if SkipCompilerExecution is true dotnet/BenchmarkDotNet#3252, and theUseMonoRuntimerevert). Almost the entire diff on this PR is that merge; the changes themselves are the second commit, 18 files.Fix the csproj build plumbing's output paths and reference gathering— the actual changes. The commit message has the full reasoning; in outline:ForcedNoDependenciesForIntegrationTestsgate, fixesGetPublishDirectoryPath, and setsOutDir/OutputPath/AppendTargetFrameworkToOutputPathin the generated project rather than derivingOutputPathby trimming the TFM off the binaries path.@(ReferencePath)instead of globbing*.dllout of the benchmark project's output. That drops theMSB3246,NETSDK1152and missing-exe-assembly cases at once, so bothErrorOnDuplicatePublishOutputFilessuppressions go.DllGatherer.csprojis gone: the gathering pass builds the generated project itself withSkipCompilerExecution, so it needs no second project and no stub entry point.XDocumentwith overridable hooks instead of substituted into text templates. All five templates are deleted; R2R, Wasm and NativeAOT had each kept a drifted near-copy of the default one.dotnet publishrather than restore + build + publish./p:ArtifactsPathas a property rather than--artifacts-path, so no .NET 8 SDK floor andnet7and earlier keep master's sequential fallback.🤖 Generated with Claude Code