allow affinity mask wider than 32 processors - #3242
Conversation
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
There was a problem hiding this comment.
Pull request overview
This PR widens the --affinity CLI option to accept masks beyond 32 bits, aligning the console entry point with the rest of the affinity pipeline (which already uses IntPtr and masks up to 64 bits).
Changes:
- Change
CommandLineOptions.Affinityfromint?tolong?so higher-bit masks can be parsed from the CLI. - Update Perfonar environment projection (
EnvironmentMode.ToPerfonar()/BdnEnvironment) to avoid truncation/overflow when affinity exceedsint. - Add regression tests to ensure affinity parsing works for both small masks and a mask with a bit above 32.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/BenchmarkDotNet.Tests/Perfonar/PerfonarTests.cs | Updates test helper signature to accept long? affinity for Perfonar output generation. |
| tests/BenchmarkDotNet.Tests/ConfigParserTests.cs | Adds regression tests validating CLI affinity parsing for small and >32-bit masks. |
| src/BenchmarkDotNet/Models/BdnEnvironment.cs | Widens internal Perfonar environment model affinity from int? to long? to prevent truncation/overflow. |
| src/BenchmarkDotNet/Jobs/EnvironmentMode.cs | Updates affinity projection to Perfonar to use long instead of int. |
| src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs | Widens CLI option type for --affinity from int? to long?. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@dotnet-policy-service agree |
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
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
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.
|
follow up on the first copilot comment. i only fixed the test half of it and left the production half in place. copilot said the overflow hits there is a second case that a plain reject would get wrong. a full 32 processor mask is so the conversion goes through one helper that takes the pointer size: internal static bool TryConvertAffinity(ulong mask, int pointerSize, out IntPtr affinity)
{
if (pointerSize >= 8)
{
affinity = new IntPtr(unchecked((long)mask));
return true;
}
affinity = new IntPtr(unchecked((int)mask));
return mask <= uint.MaxValue;
}
passing the pointer size in is what makes the 32 bit path testable on a 64 bit machine. three theories in
with the helper body swapped back to the old unguarded put back it is 142 of 142 with 3 unrelated skips. ran what
net10.0 on macos arm64. one more thing while i was in here. i wanted to back up my own claim about leaving and called so the perfonar model does have to stay signed until perfolizer grows a |
problem
--affinityis parsed asintso any mask with a bit above 32 can not be passed at all.everything under it is already wider. the job stores affinity as
IntPtrandFixAffinityinProcessExtensionsalready masks against a 64 bitcpuMaskand comments that the max supported affinity without cpu groups is 64. only the cli entry point was narrow.fix
int?becomesulong?on the option. unsigned so the 64th cpu is written as9223372036854775808the way the mask reads rather than as-9223372036854775808. thanks @timcassell for pushing for that.three things had to move with it.
the perfonar projection.
EnvironmentMode.ToPerfonar()didonce a wider mask can actually reach that line the cast is a problem. on net10
IntPtrisnintso it truncates silently and on net472 the explicit operator goes throughToInt32()which throwsOverflowException. soBdnEnvironment.Affinitybecomeslong?and the cast becomes(long).BdnEnvironmentis internal so this is not a public api change.it stays signed rather than unsigned on purpose. perfolizer 0.7.5
LightJsonSerializerhasAppendInt(Int32)andAppendLong(Int64)andAppendDouble(Double)and noUInt64case so aulongthere throwsNotSupportedException: Unsupported type: System.UInt64. same bits either way.the conversion to IntPtr.
new IntPtr(long)ischecked((int)value)on a 32 bit process so it throws instead of truncating. that would have madeConfigParser.Parsefail with an unhandledOverflowExceptionon x86 rather than a normal option error. a full 32 processor mask of0xFFFFFFFFdoes not fit ininteither even though it is legal there. so the conversion goes through a helper that takes the pointer sizewhich keeps the whole 64 bit mask on 8 bytes and takes the low 32 bits on 4 bytes and reports failure for anything wider so
Validatecan print an error. on a 64 bit process it is bit for bit the same as before.a new env requirement.
EnvRequirement.Platform64BitOnlyso the wide mask parse tests skip rather than fail on a 32 bit runtime.tests
in
ConfigParserTestsUserCanSpecifyAffinityfor a normal small maskUserCanSpecifyAffinityBeyondThirtyTwoProcessorsfor1UL << 40UserCanSpecifyAffinityForTheSixtyFourthProcessorfor1UL << 63AffinityThatFitsThirtyTwoBitsIsAcceptedByAThirtyTwoBitProcessAffinityWiderThanThirtyTwoBitsIsRejectedByAThirtyTwoBitProcessAffinityOfAnyWidthIsAcceptedByASixtyFourBitProcessthe last three take the pointer size as an argument so the 32 bit behaviour is checked on any machine.
they are real regression tests. with the option reverted to
int?the wide mask test fails and the small one still passes. with the pointer size guard removed five of the nine theory cases fail.verification
./build.cmd buildand./build.cmd unit-tests -eand./build.cmd analyzer-tests -ewhich is whatrun-tests.yamlruns.net10.0 on macos arm64.
note
i did not touch cpu groups. this only widens what the cli can express up to the 64 that
FixAffinityalready supports.closes #3231