Improved performance for SemaphoreSlim locking.

[MarkCiliaVincenti]

No description provided.

[maliming]

hi @MarkCiliaVincenti

Thanks for your contribution. 👍

Can you use source code instead of nuget package? This way, we can easily maintain it.

Also for:
#20531
#20530

[MarkCiliaVincenti]

This uses a NuGet by Microsoft, not sure what the problem is. It's in there due to an improvement in performance for ValueTask that was included in .NET 5.0.

#20530 will need to use an external dependency as some assembly-level attributes need to be set.

#20531 would mean copying and pasting the library.

[maliming]

If there are few code changes, we would prefer the source code rather than introducing a new Nuget package.

[MarkCiliaVincenti]

If there are few code changes, we would prefer the source code rather than introducing a new Nuget package.

Even if it's a Microsoft package?

[maliming]

I see you referenced some of your packages.

eg Backport.System.Threading.Lock, ListShuffle, ThreadSafeRandomizer

[MarkCiliaVincenti]

I see you referenced some of your packages.

eg Backport.System.Threading.Lock, ListShuffle, ThreadSafeRandomizer

Let's discuss on the appropriate PR please.

On this PR the only addition is this:

<PackageReference Include="System.Threading.Tasks.Extensions" Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net5.0'))" />

[maliming]

We have used ConfigureAwait.Fody to add ConfigureAwait(false); for each async method call.

[MarkCiliaVincenti]

We have used ConfigureAwait.Fody to add ConfigureAwait(false); for each async method call.

OK, I will update those.

[MarkCiliaVincenti]

Updated @maliming .

[maliming]

Thanks.

I think we dont need the net5.0, Almost no one is using the net5.0 and abp at the same time.

What is the purpose of System.Threading.Tasks.Extensions package?

[MarkCiliaVincenti]

Thanks.

I think we dont need the net5.0, Almost no one is using the net5.0 and abp at the same time.

What is the purpose of System.Threading.Tasks.Extensions package?

There was an improvement in performance done to ValueTask. This improvement is included in .NET 5.0. I added the target for .NET 5.0 specifically so that on .NET 5.0, .NET 6.0 and .NET 7.0 the NuGet package is not added as a dependency, otherwise:

<PackageReference Include="System.Threading.Tasks.Extensions" Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net5.0'))" />

would need to change to:

<PackageReference Include="System.Threading.Tasks.Extensions" Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net8.0'))" />

And that would mean an unnecessary dependency for .NET 5.0 to .NET 7.0.

[maliming]

I think we don't need the System.Threading.Tasks.Extensions library.

We can still use ConfigureAwait without it.

[MarkCiliaVincenti]

I think we don't need the System.Threading.Tasks.Extensions library.

We can still use ConfigureAwait without it.

The system.threading.tasks.extensions is there for valuetask and has nothing to do with configureawait.

[maliming]

The system.threading.tasks.extensions is there for valuetask and has nothing to do with configureawait.

Can you give an example of what we can get with this package?

Thanks

[MarkCiliaVincenti]

The system.threading.tasks.extensions is there for valuetask and has nothing to do with configureawait.

Can you give an example of what we can get with this package?

Thanks

ValueTask was included in netcore2.1, so to use it before then (netstandard2.0) you need to use a package. Furthermore, this PR made ValueTask more performant and was included in net5.0, so the recommendation is to reference that library for < net5.0: dotnet/coreclr#26310

[MarkCiliaVincenti]

Furthermore please note that this package is still included transitively in abp. This is just moving it at the top level.

[maliming]

ok, Thanks.

We are not using net5.0. We can add System.Threading.Tasks.Extensions for netstandard2.0

<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
  <PackageReference Include="System.Threading.Tasks.Extensions" />
</ItemGroup>

[MarkCiliaVincenti]

If we do that then netcore 3.0 and 3.1 would have the unoptimal version. Are you sure you want this, code that performs better in netcore 2.1 than it does in netcore 3.1?

[MarkCiliaVincenti]

So basically you have 3 options:

1. have less performant netcore 3.x
2. include the library also for netstandard2.1, but this would add an unnecessary dependency for net5, net6 and net7.
3. target net5 just from the core lib as this PR is doing.

[maliming]

Almost no one uses ABP 9.x and <=net5 at the same time. and they can also reference this package themselves to improve performance.

I think adding it to netstandard2.0 is enough.

<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
  <PackageReference Include="System.Threading.Tasks.Extensions" />
</ItemGroup>

[MarkCiliaVincenti]

OK now it's as requested @maliming

[maliming]

Thanks!