Skip to content

Commit d2b612f

Browse files
committed
Remove InProcess check from validation.
Update Weaver.
1 parent 5857773 commit d2b612f

File tree

11 files changed

+42
-71
lines changed

11 files changed

+42
-71
lines changed

src/BenchmarkDotNet.Weaver/BenchmarkDotNet.Weaver.csproj

+6-4
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
<Project Sdk="Microsoft.NET.Sdk">
55
<Import Project="..\..\build\common.props" />
66
<PropertyGroup>
7-
<TargetFrameworks>net461;netstandard2.0</TargetFrameworks>
8-
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
9-
<OutputPath>$(MSBuildThisFileDirectory)bin\$(Configuration)</OutputPath>
7+
<TargetFrameworks>netstandard2.0</TargetFrameworks>
108
<VersionSuffix Condition="'$(IsFullPack)' != 'true'">$(VersionSuffix)$(WeaverVersionSuffix)</VersionSuffix>
9+
<OutputPath>$(MSBuildThisFileDirectory)bin\$(Configuration)</OutputPath>
10+
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
11+
<IncludeBuildOutput>false</IncludeBuildOutput>
12+
<NoWarn>$(NoWarn);NU5100;NU5128</NoWarn>
1113
</PropertyGroup>
1214

1315
<ItemGroup>
@@ -19,7 +21,7 @@
1921
<ItemGroup>
2022
<!-- Include .targets file and all DLLs in the output directory in the NuGet package -->
2123
<Content Include="$(MSBuildThisFileDirectory)buildTransitive\**\*.targets" Pack="true" PackagePath="buildTransitive" />
22-
<Content Include="$(OutputPath)**\*.dll" Exclude="$(OutputPath)**\$(AssemblyName).dll" Pack="true" PackagePath="lib/$(TargetFramework)" />
24+
<Content Include="$(OutputPath)**\*.dll" Pack="true" PackagePath="tasks/$(TargetFramework)" />
2325
<None Remove="packages\**" />
2426
</ItemGroup>
2527
</Project>

src/BenchmarkDotNet.Weaver/buildTransitive/net461/BenchmarkDotNet.Weaver.targets

-10
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3-
<UsingTask TaskName="BenchmarkDotNet.Weaver.WeaveAssembliesTask" AssemblyFile="$(MSBuildThisFileDirectory)..\..\lib\netstandard2.0\BenchmarkDotNet.Weaver.dll" />
3+
<UsingTask TaskName="BenchmarkDotNet.Weaver.WeaveAssemblyTask" AssemblyFile="$(MSBuildThisFileDirectory)..\..\tasks\netstandard2.0\BenchmarkDotNet.Weaver.dll" />
44

5-
<!-- .Net Framework causes "warning MSB3026: Could not copy ... The file is locked by: .NET Host" when the weaver task is ran on the user's project
6-
when the wrapper project is built. So DotNetCliCommand disables it globally, and the wrapper project enables it only for itself. -->
7-
<Target Name="WeaveAssemblies" AfterTargets="AfterBuild" Condition="'$(BenchmarkDotNetSkipWeaver)' != 'true' Or '$(BenchmarkDotNetWrapperProj)' == 'true'">
8-
<WeaveAssembliesTask TargetDir="$(TargetDir)" TargetAssembly="$(TargetDir)$(TargetFileName)" />
5+
<Target Name="WeaveAssemblies" AfterTargets="AfterBuild" >
6+
<WeaveAssemblyTask TargetDir="$(TargetDir)" TargetAssembly="$(TargetDir)$(TargetFileName)" />
97
</Target>
108
</Project>
Binary file not shown.
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Collections.Generic;
32
using System.IO;
43
using System.Linq;
54
using Microsoft.Build.Framework;
@@ -10,17 +9,19 @@ namespace BenchmarkDotNet.Weaver;
109

1110
internal class CustomAssemblyResolver : DefaultAssemblyResolver
1211
{
13-
public override AssemblyDefinition Resolve(AssemblyNameReference name, ReaderParameters parameters)
14-
// Fix StackOverflow https://github.com/jbevain/cecil/issues/573
15-
=> name.FullName.StartsWith("netstandard") || name.FullName.StartsWith("mscorlib") || name.FullName.StartsWith("System.Private.CoreLib")
16-
? AssemblyDefinition.ReadAssembly(Path.Combine(Path.GetDirectoryName(typeof(object).Assembly.Location), Path.ChangeExtension(name.Name, ".dll")), parameters)
17-
: base.Resolve(name, parameters);
12+
public override AssemblyDefinition Resolve(AssemblyNameReference name, ReaderParameters parameters)
13+
// NetStandard causes StackOverflow. https://github.com/jbevain/cecil/issues/573
14+
// Mscorlib fails to resolve in Visual Studio. https://github.com/jbevain/cecil/issues/966
15+
// We don't care about any types from these assemblies anyway, so just skip resolving them.
16+
=> name.Name is "netstandard" or "mscorlib" or "System.Runtime" or "System.Private.CoreLib"
17+
? null// AssemblyDefinition.ReadAssembly(Path.Combine(Path.GetDirectoryName(typeof(object).Assembly.Location), Path.ChangeExtension(name.Name, ".dll")))
18+
: base.Resolve(name, parameters);
1819
}
1920

2021
/// <summary>
2122
/// The Task used by MSBuild to weave the assemblies.
2223
/// </summary>
23-
public sealed class WeaveAssembliesTask : Task
24+
public sealed class WeaveAssemblyTask : Task
2425
{
2526
/// <summary>
2627
/// The directory of the output.
@@ -29,13 +30,11 @@ public sealed class WeaveAssembliesTask : Task
2930
public string TargetDir { get; set; }
3031

3132
/// <summary>
32-
/// The path of the target assemblies.
33+
/// The path of the target assembly.
3334
/// </summary>
3435
[Required]
3536
public string TargetAssembly { get; set; }
3637

37-
private readonly List<string> _warningMessages = [$"Benchmark methods were found in 1 or more assemblies that require NoInlining, and assembly weaving failed."];
38-
3938
/// <summary>
4039
/// Runs the weave assemblies task.
4140
/// </summary>
@@ -49,44 +48,20 @@ public override bool Execute()
4948
}
5049

5150
var resolver = new CustomAssemblyResolver();
52-
resolver.AddSearchDirectory(TargetDir);
51+
resolver.AddSearchDirectory(TargetDir);
5352

5453
// ReaderParameters { ReadWrite = true } is necessary to later write the file.
5554
// https://stackoverflow.com/questions/41840455/locked-target-assembly-with-mono-cecil-and-pcl-code-injection
5655
var readerParameters = new ReaderParameters
5756
{
5857
ReadWrite = true,
5958
AssemblyResolver = resolver
60-
};
61-
62-
ProcessAssembly(TargetAssembly, readerParameters, out bool isExecutable);
63-
64-
foreach (var assemblyPath in Directory.GetFiles(TargetDir, "*.dll"))
65-
{
66-
if (assemblyPath == TargetAssembly)
67-
{
68-
continue;
69-
}
70-
ProcessAssembly(assemblyPath, readerParameters, out _);
71-
}
72-
73-
// Assembly resolution can fail for library projects that contain references if the project does not have <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>.
74-
// Because the library project could be built as a dependency of the executable, we only log the warning if the target assembly is executable.
75-
if (_warningMessages.Count > 1 && isExecutable)
76-
{
77-
Log.LogWarning(string.Join(Environment.NewLine, _warningMessages));
78-
}
79-
return true;
80-
}
81-
82-
private void ProcessAssembly(string assemblyPath, ReaderParameters readerParameters, out bool isExecutable)
83-
{
84-
isExecutable = false;
59+
};
60+
8561
bool benchmarkMethodsImplAdjusted = false;
8662
try
8763
{
88-
using var module = ModuleDefinition.ReadModule(assemblyPath, readerParameters);
89-
isExecutable = module.EntryPoint != null;
64+
using var module = ModuleDefinition.ReadModule(TargetAssembly, readerParameters);
9065

9166
foreach (var type in module.Types)
9267
{
@@ -99,10 +74,11 @@ private void ProcessAssembly(string assemblyPath, ReaderParameters readerParamet
9974
catch (Exception e)
10075
{
10176
if (benchmarkMethodsImplAdjusted)
102-
{
103-
_warningMessages.Add($"Assembly: {assemblyPath}, error: {e.Message}");
77+
{
78+
Log.LogWarning($"Benchmark methods were found that require NoInlining, and assembly weaving failed.{Environment.NewLine}{e}");
10479
}
10580
}
81+
return true;
10682
}
10783

10884
private void ProcessType(TypeDefinition type, ref bool benchmarkMethodsImplAdjusted)
@@ -116,7 +92,7 @@ private void ProcessType(TypeDefinition type, ref bool benchmarkMethodsImplAdjus
11692
// Remove AggressiveInlining and add NoInlining to all [Benchmark] methods.
11793
foreach (var method in type.Methods)
11894
{
119-
if (method.CustomAttributes.Any(attr => attr.AttributeType.FullName == "BenchmarkDotNet.Attributes.BenchmarkAttribute"))
95+
if (method.CustomAttributes.Any(IsBenchmarkAttribute))
12096
{
12197
var oldImpl = method.ImplAttributes;
12298
method.ImplAttributes = (oldImpl & ~MethodImplAttributes.AggressiveInlining) | MethodImplAttributes.NoInlining;
@@ -130,4 +106,17 @@ private void ProcessType(TypeDefinition type, ref bool benchmarkMethodsImplAdjus
130106
ProcessType(nestedType, ref benchmarkMethodsImplAdjusted);
131107
}
132108
}
109+
110+
private bool IsBenchmarkAttribute(CustomAttribute attribute)
111+
{
112+
// BenchmarkAttribute is unsealed, so we need to walk its hierarchy.
113+
for (var attr = attribute.AttributeType; attr != null; attr = attr.Resolve()?.BaseType)
114+
{
115+
if (attr.FullName == "BenchmarkDotNet.Attributes.BenchmarkAttribute")
116+
{
117+
return true;
118+
}
119+
}
120+
return false;
121+
}
133122
}

src/BenchmarkDotNet/Templates/CsProj.txt

-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
<ValidateExecutableReferencesMatchSelfContained>false</ValidateExecutableReferencesMatchSelfContained>
2929
<!-- Suppress warning for nuget package used in old (unsupported) tfm. -->
3030
<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>
31-
<BenchmarkDotNetWrapperProj>true</BenchmarkDotNetWrapperProj>
3231
<StartupObject>BenchmarkDotNet.Autogenerated.UniqueProgramName</StartupObject>
3332
</PropertyGroup>
3433

src/BenchmarkDotNet/Templates/MonoAOTLLVMCsProj.txt

-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
<SelfContained>true</SelfContained>
2323
<!-- Suppress warning for nuget package used in old (unsupported) tfm. -->
2424
<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>
25-
<BenchmarkDotNetWrapperProj>true</BenchmarkDotNetWrapperProj>
2625
</PropertyGroup>
2726

2827
<ItemGroup>

src/BenchmarkDotNet/Templates/WasmCsProj.txt

-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
<EnableDefaultWasmAssembliesToBundle>false</EnableDefaultWasmAssembliesToBundle>
2929
<!-- Suppress warning for nuget package used in old (unsupported) tfm. -->
3030
<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>
31-
<BenchmarkDotNetWrapperProj>true</BenchmarkDotNetWrapperProj>
3231
<StartupObject>BenchmarkDotNet.Autogenerated.UniqueProgramName</StartupObject>
3332
</PropertyGroup>
3433

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs

+2-5
Original file line numberDiff line numberDiff line change
@@ -218,17 +218,14 @@ private static string GetMandatoryMsBuildSettings(string buildConfiguration)
218218
// we use these settings to make sure that MSBuild does the job and simply quits without spawning any long living processes
219219
// we want to avoid "file in use" and "zombie processes" issues
220220
const string NoMsBuildZombieProcesses = "/p:UseSharedCompilation=false /p:BuildInParallel=false /m:1 /p:Deterministic=true";
221-
// .Net Framework causes "warning MSB3026: Could not copy ... The file is locked by: .NET Host" when the weaver task is ran
222-
// on the user's project when the wrapper project is built. So we disable it globally, and enable it only for the wrapper project.
223-
const string SkipWeaver = "/p:BenchmarkDotNetSkipWeaver=true";
224221
const string EnforceOptimizations = "/p:Optimize=true";
225222

226223
if (string.Equals(buildConfiguration, RuntimeInformation.DebugConfigurationName, StringComparison.OrdinalIgnoreCase))
227224
{
228-
return $"{NoMsBuildZombieProcesses} {SkipWeaver}";
225+
return NoMsBuildZombieProcesses;
229226
}
230227

231-
return $"{NoMsBuildZombieProcesses} {EnforceOptimizations} {SkipWeaver}";
228+
return $"{NoMsBuildZombieProcesses} {EnforceOptimizations}";
232229
}
233230

234231
private static string BuildAddPackageCommand(NuGetReference reference)

src/BenchmarkDotNet/Toolchains/NativeAot/Generator.cs

-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ private string GenerateProjectForNuGetBuild(BuildPartition buildPartition, Artif
144144
<ErrorOnDuplicatePublishOutputFiles>false</ErrorOnDuplicatePublishOutputFiles> <!-- workaround for 'Found multiple publish output files with the same relative path.' error -->
145145
<ValidateExecutableReferencesMatchSelfContained>false</ValidateExecutableReferencesMatchSelfContained>
146146
<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> <!-- Suppress warning for nuget package used in old (unsupported) tfm. -->
147-
<BenchmarkDotNetWrapperProj>true</BenchmarkDotNetWrapperProj>
148147
{GetInstructionSetSettings(buildPartition)}
149148
</PropertyGroup>
150149
{GetRuntimeSettings(buildPartition.RepresentativeBenchmarkCase.Job.Environment.Gc, buildPartition.Resolver)}

src/BenchmarkDotNet/Validators/CompilationValidator.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,7 @@ private static IEnumerable<ValidationError> ValidateBindingModifiers(IEnumerable
9797
));
9898

9999
private static IEnumerable<ValidationError> ValidateMethodImpl(IEnumerable<BenchmarkCase> benchmarks)
100-
// Only InProcess toolchains need this validation. Build toolchains run the assembly weaver to automatically apply the MethodImpl flag.
101-
=> benchmarks.Where(x => !x.Descriptor.WorkloadMethod.MethodImplementationFlags.HasFlag(MethodImplAttributes.NoInlining) && x.GetToolchain().IsInProcess)
100+
=> benchmarks.Where(x => !x.Descriptor.WorkloadMethod.MethodImplementationFlags.HasFlag(MethodImplAttributes.NoInlining))
102101
.Distinct(BenchmarkMethodEqualityComparer.Instance)
103102
.Select(benchmark
104103
=> new ValidationError(

0 commit comments

Comments
 (0)