Allow for Config per method, introduce OS and OSArchitecture filters #1097
Conversation
| : base(new SimpleFilter(_ => | ||
| { | ||
| return allowed | ||
| ? platforms.Any(platform => RuntimeInformation.IsOSPlatform(Map(platform))) |
There was a problem hiding this comment.
Maybe you could take the string of a OSPlatform instead of PlatformID? (basically the string that gets passed into https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.osplatform.create)
I don't think PlatformID is going to work.
From https://docs.microsoft.com/en-us/dotnet/api/system.platformid
MacOSX | 6 | The operating system is Macintosh. This value was returned by Silverlight. On .NET Core, its replacement is Unix.
From that, it sounds like you won't be able to distinguish between macOS and Linux on .NET Core using PlatformID.
There was a problem hiding this comment.
@eerhardt I am not sure about string. For example: let's say that I dont't want to run the benchmark on Ubuntu. Should I put Unix? Linux? or Ubuntu? or Ubuntu16.04?
But I agree that PlatformID is not perfect, I had to implement a mapping on my own.
Maybe I should just introduce a new enum?
There was a problem hiding this comment.
Should I put Unix? Linux? or Ubuntu? or Ubuntu16.04?
Do you want that deep of control? Do you want to be able to say "All Unixes" vs. "just Ubuntu, but not RedHat or OpenSUSE"? If so, it seems like a perfect place to use RIDs...
There was a problem hiding this comment.
Do you want that deep of control?
No, I just wanted to show that without an enum, the user needs to guess the right string value.
There was a problem hiding this comment.
It's been two years since I created this PR, but now I really need tha for Randomization in the perf repo so I've updated it. I decided to include my own simple enum, just so users don't have to type OS names manually (an avoid mistakes for "OSX" != "macOS" etc)
# Conflicts: # src/BenchmarkDotNet/Attributes/Filters/FilterConfigBaseAttribute.cs # src/BenchmarkDotNet/Running/BenchmarkConverter.cs # src/BenchmarkDotNet/Running/BenchmarkRunnerDirty.cs
in the dotnet/performance repo, I need to enable/disable some benchmarks per OS/architecture/something else like AVX2.IsAvailable. As of today, we allow to enable the configs per entire type, so if I want to disable one benchmark out of N for Linux, I have to move it to a dedicated type and apply some config to it via attributes. This is not very user friendly.
My proposal: allow for configs per methods.
I have changed how
BenchmarkConverterworks and now it looks for the method attributes as well.What is very important, I did not change
AttributeTargetsflags for any of our Attributes except ofFilterConfigBaseAttributeandJobMutatorConfigBaseAttributeso basically this feature currently works only for Filter and Job attributes.@AndreyAkinshin what do you think?
/cc @ig-sinicyn