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

Running benchmark fails when targeting netcoreapp2.2 #995

Closed
jasper-d opened this issue Jan 4, 2019 · 13 comments
Closed

Running benchmark fails when targeting netcoreapp2.2 #995

jasper-d opened this issue Jan 4, 2019 · 13 comments
Assignees
Milestone

Comments

@jasper-d
Copy link

jasper-d commented Jan 4, 2019

Running a benchmark fails when targeting netcoreapp2.2. That is because the *.Autogenerated.csproj. targets netcoreapp2.1 and references the original benchmark project that targets a higher netcoreapp version.

Repro:

  1. Create a new netcore CLI application that targets netcoreapp2.2
  2. Add a reference to BenchmarkDotNet v0.11.3
  3. Add any Benchmark w/o explicit configuration (the Md5VsSha256 example from the docs will do)
  4. Build the benchmark project: dotnet build -f netcoreapp2.2 -c Release -r win-x64 --no-incremental --force
  5. Run the benchmark like so: .\bin\Release\netcoreapp2.2\win-x64\Target22.exe

Expected:
The benchmark completes without errors.

Actual:
Running the benchmark (repro step 5) fails early when building the autogenerated code because the autogenerated csproj targets netcoreapp2.1 and references the original projects which targets 2.2.

Workaround:
Use an explicit configuration like so;

    private class DotNetCore22Config : ManualConfig
    {
        public DotNetCore22Config()
        {
            Add(Job.Default.With(CsProjCoreToolchain.NetCoreApp22));
        }
    }

Let me know if you need a complete sample project to reproduce.

@adamsitnik
Copy link
Member

Hi @jasper-d

Could you run following code and share the output?

private static string GetBaseNetCoreVersion()
{
    var assembly = typeof(GCSettings).GetTypeInfo().Assembly;
    var assemblyPath = assembly.CodeBase.Split(new[] { '/', '\\' }, StringSplitOptions.RemoveEmptyEntries);
    int netCoreAppIndex = Array.IndexOf(assemblyPath, "Microsoft.NETCore.App");
    if (netCoreAppIndex > 0 && netCoreAppIndex < assemblyPath.Length - 2)
        return assemblyPath[netCoreAppIndex + 1];
    return null;            
}

(this is the code we use to detect .NET Core version)

@jasper-d
Copy link
Author

jasper-d commented Jan 4, 2019

Hi @adamsitnik, that method returns 2.2.0.

@jasper-d
Copy link
Author

jasper-d commented Jan 4, 2019

Ok, some more information.

When running the benchmark from the command line the assembly path in GetBaseNetCoreVersion is different from running from within VS.
It then is C:/Users/jasper-d/.nuget/packages/runtime.win-x64.microsoft.netcore.app/2.2.0/runtimes/win-x64/native/System.Private.CoreLib.dll. Subsequently netCoreAppIndex equals -1 and null is returned. GetCurrentVersion then defaults to NetCoreApp2.1

@jasper-d
Copy link
Author

jasper-d commented Jan 4, 2019

Is there a reason the corelib path is parsed instead of using Assembly.GetEntryAssembly().GetCustomAttribute<TargetFrameworkAttribute>().FrameworkName or something similar to get the core runtime version?

@adamsitnik
Copy link
Member

@jasper-d I don't remember if we had any reason for that.

Does Assembly.GetEntryAssembly().GetCustomAttribute<TargetFrameworkAttribute>().FrameworkName return a proper value for all the scenarios? If so we can switch to it. (it sounds way more natural to use than what we have today)

@adamsitnik
Copy link
Member

@jasper-d would you like to send a PR?

@jasper-d
Copy link
Author

jasper-d commented Jan 7, 2019

@adamsitnik Yes, will do so tonight. But could you elaborate on the scenarios you mentioned?

@adamsitnik
Copy link
Member

But could you elaborate on the scenarios you mentioned?

@jasper-d please just make sure that it works on .NET Core 2.0, 2.1, 2.2 and 3.0 ;)

@jasper-d
Copy link
Author

jasper-d commented Jan 7, 2019

@adamsitnik Fortunately it does (even with CoreRT).
I'm just testing if it works on Docker too (it should). I would then remove the special handling of Docker environments.

@adamsitnik
Copy link
Member

@jasper-d awesome!

jasper-d added a commit to jasper-d/BenchmarkDotNet that referenced this issue Jan 8, 2019
@mattzink
Copy link

mattzink commented Jan 16, 2019

TargetFrameworkAttribute returns the static framework version (TFM) that the app was compiled for, and therefore is not the runtime version that is actually executing your code. Is this what you guys are after? I would have thought the runtime version is more useful.

Also, I would think you would want the full runtime version (i.e. 2.2.1) since there can be important behavioral/performance differences between patch releases.

@patricksuo
Copy link

@jasper-d @adamsitnik do you know any workaround for this?

@jasper-d
Copy link
Author

@sillyousu See the workaround section above (i.e. create a custom ManualConfig that specifies the runtime) and use it as documented.

@mattzink Here the version is used during code generation to set the TFM in a generated .csproj. So yes, we wanna know what framework an application was build for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants