Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to recompile for each target runtime #1773

Closed
stephentoub opened this issue Aug 15, 2021 · 15 comments · Fixed by #2370
Closed

Ability to recompile for each target runtime #1773

stephentoub opened this issue Aug 15, 2021 · 15 comments · Fixed by #2370
Milestone

Comments

@stephentoub
Copy link
Member

Today in order to compare runtimes, I need to build against a single runtime (the lowest common denominator) with -f and then I can execute against multiple runtimes with --runtimes. That works in the majority use case, however sometimes you want to answer the question "how does this code compare on different runtimes?", and that includes compile-time impact due to new overloads being introduced, which necessitates building against the appropriate reference assemblies for each version.

@timcassell
Copy link
Collaborator

Possibly related? It would be nice to be able to recompile targets with different versions of a library/dll. Currently, I can use 2 different versions of the same library in separate projects, both projects referenced by the benchmark project and each called with a different benchmark method, and that compiles just fine, but fails at runtime with System.Reflection.TargetInvocationException.

I'm not sure how that could be achieved, considering I have to include the references in the csproj itself.

@timcassell
Copy link
Collaborator

This is already working for Core runtimes. I sent a PR to fix this for Framework in #2370. I'm also aware of classic Mono not being able to rebuild per target framework, but I'm not sure if we need to add that support. Do you need that, and are there any other runtimes that need this and are not already being rebuilt @stephentoub?

@stephentoub
Copy link
Member Author

core and framework are the two I've personally cared about.

@timcassell timcassell added this to the v0.13.7 milestone Jul 27, 2023
@stephentoub
Copy link
Member Author

@timcassell, can you clarify what you mean about this already working for Core? I'm not seeing that. I see a project get build that targets the original .dll and whatever target it was built for.

@timcassell
Copy link
Collaborator

@stephentoub I added tests showing that we are recompiling. Can you share a repro of what you are seeing?

@stephentoub
Copy link
Member Author

nuget.config:

<configuration>
  <packageSources>
    <add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" />
  </packageSources>
</configuration>

Program.cs

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
using Microsoft.Extensions.Configuration;
using System;

var config = DefaultConfig.Instance
    .AddJob(Job.Default.WithRuntime(CoreRuntime.Core70).WithId(".NET 7.0").WithNuGet("Microsoft.Extensions.Configuration", "7.0.0").AsBaseline())
    .AddJob(Job.Default.WithRuntime(CoreRuntime.Core80).WithId(".NET 8.0").WithNuGet("Microsoft.Extensions.Configuration", "8.0.0-rc.1.23402.3"))
    .AddJob(Job.Default.WithRuntime(CoreRuntime.Core80)
        .WithNuGet("Microsoft.Extensions.Configuration", "8.0.0-rc.1.23402.3")
        .WithNuGet("Microsoft.Extensions.Configuration.Binder", "8.0.0-rc.1.23402.3")
        .WithArguments(new Argument[] { new MsBuildArgument("/p:EnableConfigurationBindingGenerator=true") }));
BenchmarkSwitcher.FromAssembly(typeof(Tests).Assembly).Run(args, config);

[HideColumns("Error", "StdDev", "Median", "RatioSD", "NuGetReferences")]
[MemoryDiagnoser(false)]
public partial class Tests
{
    private MyConfigSection _data = new MyConfigSection();
    private IConfiguration _config;

    [GlobalSetup]
    public void Setup()
    {
        Environment.SetEnvironmentVariable("MyConfigSection__Message", "Hello World!");
        _config = new ConfigurationBuilder()
            .AddEnvironmentVariables()
            .Build();
    }

    [Benchmark]
    public void Load() => _config.Bind("MyConfigSection", _data);

    internal sealed class MyConfigSection
    {
        public string Message { get; set; }
    }
}

Benchmarks.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net8.0;net7.0</TargetFrameworks>
    <LangVersion>Preview</LangVersion>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <ServerGarbageCollection>true</ServerGarbageCollection>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="benchmarkdotnet" Version="0.13.6" />
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="7.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="7.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0-rc.1.23402.3" Condition=" '$(TargetFramework)' == 'net8.0' " />
  </ItemGroup>

</Project>

Then run with:

dotnet run -c Release -f net7.0 --filter **
Method Job Runtime Arguments Mean Ratio Allocated Alloc Ratio
Load .NET 7.0 .NET 7.0 Default 2.828 us 1.00 1.3 KB 1.00
Load .NET 8.0 .NET 8.0 Default 2.097 us 0.74 1.37 KB 1.05
Load Job-UWBTBT .NET 8.0 /p:EnableConfigurationBindingGenerator=true 2.045 us 0.72 1.37 KB 1.05

That last line should be closer to 60ns if it were actually being compiled as desired, and if you add <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator> to the PropertyGroup in the csproj, you'll see it drops to that (but so does the other .NET 8 row).

@timcassell
Copy link
Collaborator

Oh I see, that's a separate issue. #1377

@stephentoub
Copy link
Member Author

Oh I see, that's a separate issue. #1377

If I --keepFiles, I see the supplied argument being passed to the dotnet build command in the generated .bat file, but that's not the project that needs the argument.

@timcassell
Copy link
Collaborator

Do you know how to pass it to the proper project? I couldn't figure it out. When I tried it without BDN, it was passed to both projects, but with BDN it doesn't seem to work.

@stephentoub
Copy link
Member Author

stephentoub commented Aug 2, 2023

Nope 😦

@adamsitnik
Copy link
Member

I commented out all job definitions except the last one and it worked as expected. I assumed that the build issue was caused by the fact that 3 jobs were build in parallel (this is was BDN does by default), and simply the non-source generator net8.0 .dll got consumed by two .NET 8 jobs.

So I modified BDN to disable parallel builds, just to find out it does not solve the problem.

So I've used an old feature: specify build configuration per job via WithCustomBuildConfiguration, move all the MSBuild magic to csproj. And it works fine:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net8.0;net7.0</TargetFrameworks>
    <LangVersion>Preview</LangVersion>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <ServerGarbageCollection>true</ServerGarbageCollection>
    <EnableConfigurationBindingGenerator Condition=" '$(Configuration)' == 'Sourcegen' ">true</EnableConfigurationBindingGenerator>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="benchmarkdotnet" Version="0.13.6" />
    <PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="7.0.0" Condition=" '$(TargetFramework)' == 'net7.0' "/>
	<PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="8.0.0-rc.1.23402.3" Condition=" '$(TargetFramework)' == 'net8.0' "/>
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.0-rc.1.23402.3" Condition=" '$(TargetFramework)' == 'net8.0' "/>
  </ItemGroup>

</Project>
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
using Microsoft.Extensions.Configuration;
using System;

var config = DefaultConfig.Instance
    .AddJob(Job.Default.WithRuntime(CoreRuntime.Core70).WithId(".NET 7.0").AsBaseline())
    .AddJob(Job.Default.WithRuntime(CoreRuntime.Core80).WithId(".NET 8.0"))
    .AddJob(Job.Default.WithRuntime(CoreRuntime.Core80)
        .WithId(".NET 8.0 Sourcegen")
        .WithCustomBuildConfiguration("Sourcegen"));

BenchmarkSwitcher.FromAssembly(typeof(Tests).Assembly).Run(args, config);

[HideColumns("Error", "StdDev", "Median", "RatioSD", "NuGetReferences")]
[MemoryDiagnoser(false)]
public partial class Tests
{
    private MyConfigSection _data = new MyConfigSection();
    private IConfiguration _config;

    [GlobalSetup]
    public void Setup()
    {
        Environment.SetEnvironmentVariable("MyConfigSection__Message", "Hello World!");
        _config = new ConfigurationBuilder()
            .AddEnvironmentVariables()
            .Build();
    }

    [Benchmark]
    public void Load() => _config.Bind("MyConfigSection", _data);

    internal sealed class MyConfigSection
    {
        public string Message { get; set; }
    }
}
BenchmarkDotNet v0.13.6, Windows 11 (10.0.22621.1992/22H2/2022Update/SunValley2)
AMD Ryzen Threadripper PRO 3945WX 12-Cores, 1 CPU, 24 logical and 12 physical cores
.NET SDK 8.0.100-preview.6.23330.14
  [Host]             : .NET 7.0.9 (7.0.923.32018), X64 RyuJIT AVX2
  .NET 7.0           : .NET 7.0.9 (7.0.923.32018), X64 RyuJIT AVX2
  .NET 8.0           : .NET 8.0.0 (8.0.23.32907), X64 RyuJIT AVX2
  .NET 8.0 Sourcegen : .NET 8.0.0 (8.0.23.32907), X64 RyuJIT AVX2
Method Job Runtime BuildConfiguration Mean Ratio Allocated Alloc Ratio
Load .NET 7.0 .NET 7.0 Default 2,044.77 ns 1.00 1328 B 1.00
Load .NET 8.0 .NET 8.0 Default 1,458.20 ns 0.71 1400 B 1.05
Load .NET 8.0 Sourcegen .NET 8.0 Sourcegen 57.50 ns 0.03 112 B 0.08

@timcassell
Copy link
Collaborator

timcassell commented Aug 4, 2023

I commented out all job definitions except the last one and it worked as expected.

Really? I tried that and got the same result as the job without the MsBuildArgument.

[Edit] Actually, I tried again after I cleared my bin, and I got expected results. That's telling of something...

[Edit2] Well, now every time I run it with only the last job, it works as expected, and I can't repro what I was seeing before. :/

[Edit3] I tried some stuff, then reverted everything, now every run is the slower result. How strange.

[Edit4] Oh, it depends what my host runtime is. If I run it with host as net7.0, it runs fast, but if I run it with host as net8.0, it runs slow. I'm guessing it's trying to overwrite the built exe, but because it's running, it doesn't.

@timcassell
Copy link
Collaborator

timcassell commented Aug 4, 2023

I see the issue. On each build, by default the binaries are placed in ./bin/<Configuration>/<Runtime>, so with 2 builds using the same configuration and runtime, 1 of them will overwrite the other. Using a new configuration places the binaries in a new directory so it doesn't overwrite the other build.

Possible solutions:

  • Only build before the benchmarks are ran for each exe.
  • Move the binaries after each build (parallel builds will have to be disabled).
  • Build the source proj separately with manually specifying the binaries output path, then build the auto generated csproj separately, referencing the source binaries. [Edit] This seems like the best option from my findings in my previous comment.

Honestly, I'm still trying to understand why this breaks, since I see the source binaries are copied into the job's binaries folder. I would expect disabling parallel builds to fix it, but it doesn't.

@timcassell
Copy link
Collaborator

timcassell commented Aug 4, 2023

Disabling parallel builds and using dotnet build --no-incremental works if the host is net7.0, but fails if the host is net8.0.

[Edit] I was able to get it to work in net8.0 host by also specifying a custom build configuration, which forces the binaries to a new directory. I'm not sure if there's another way we can force the source binaries to a new directory without building the source proj directly.

@timcassell
Copy link
Collaborator

I sent a fix #2393 which makes the host runtime no longer matter (unless you need classic Mono or in-process).

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 a pull request may close this issue.

3 participants