Improved locking performance on .NET 9.0+

[MarkCiliaVincenti]

The new System.Threading.Lock offers greater performance, as independent benchmarks show (eg https://steven-giesel.com/blogPost/4cf6d48a-ec9d-4c68-961c-31fd8d8c1340)

[linkdotnet]

Hey @MarkCiliaVincenti

Thanks for filing the PR. Good to know that System.Threading.Lock isn't experimental anymore.
While I am totally on board with using System.Threading.Lock for net9.0,, I am hesitant to use a backport that doesn't bring new features or crucial "polyfills."

We could do the same via:

#if NET_9_0_OR_GREATER
private readonly Lock syncLock = new();
#else
private readonly object syncLock = new();
#endif

Maybe @egil, and I can discuss this next week, but I would drop the package in favor of the more simple approach - given that we don't have many places where we are using locks.

[MarkCiliaVincenti]

Hi @linkdotnet, I didn't quite expect a comment from the guy whose blog articles helped inspire this micro-library. Thanks for that!

Yes, such an approach as you mentioned is possible, however as you are well aware of, System.Threading.Lock has some extras in it, such as the EnterScope. You will also have an issue if you try to use Monitor.Enter or the like. You should never Monitor.Enter on a Lock object, but instead call the Enter method. Essentially, not using a backport means that moving forward you will be limited in use.

[linkdotnet]

Hi @linkdotnet, I didn't quite expect a comment from the guy whose blog articles helped inspire this micro-library. Thanks for that!

Thanks for the mention and also linking the blog post as motivation for the PR.

Yes, such an approach as you mentioned is possible, however as you are well aware of, System.Threading.Lock has some extras in it, such as the EnterScope. You will also have an issue if you try to use Monitor.Enter or the like. You should never Monitor.Enter on a Lock object, but instead call the Enter method. Essentially, not using a backport means that moving forward you will be limited in use.

I am not sure if I understand 100% here. Let's dissect this a bit. We basically have two scenarios:

1. net9.0 and beyond
2. net8.0 and earlier

For the first one, sure - System.Threading.Lock will be the default lock type. So the question evolves (for me) around the latter on: net8.0 and earlier. As I understand using the backport will result in the same lowered code as with net9.0- is my assumption correct? The "only" downside is, you don't get the performance improvement (which I don't mind). Furthermore, are there any other implications for <= net8.0 given that the semantics are different.

Essentially, not using a backport means that moving forward you will be limited in use.

Why is that? Until net8.0 locking a readonly object worked just fine? I am not sure, why we are limited if we aren't using a backport. Can you elaborate on this one?

[MarkCiliaVincenti]

OK let's say you have a Lock instance and you have one method that's doing a lock (myObj) on it.

That will work with the trick you posted above. So far so good.

Now let's say you add a second method but want to have a timeout on it. Now you can neither use myObj.Enter(5) because on .NET 8.0 that won't work, nor can you use Monitor.Enter(myObj, 5) because on .NET 9.0 that has different behaviour. Remember the new locking class doesn't use Monitor, so if you try this your lock for the timeout and the normal lock (myObj) will behave as if they were two different locks, apart from not getting the performance improvement on .NET 9.0.

[MarkCiliaVincenti]

Basically:

private readonly Lock myObj = new();

void DoThis()
{
   lock (myObj)
   {
      // do something
   }
}

void DoThat()
{
   myObj.Enter(5); // this will not compile on .NET 8.0
   Monitor.Enter(myObj, 5); // this will immediately enter the lock on .NET 9.0 even if another thread is locking on DoThis()
   // do something else
}

[MarkCiliaVincenti]

Thanks for the mention and also linking the blog post as motivation for the PR.

Doesn't even stop here :)
https://www.reddit.com/r/csharp/comments/1f6ari2/locking_with_net_90s_systemthreadinglock_even_on/

[MarkCiliaVincenti]

Furthermore, are there any other implications for <= net8.0 given that the semantics are different.

The only limitation is that on .NET Framework 3.5 and .NET Framework 4.0, you cannot use the IsHeldByCurrentThread property since the IsEntered() method on Monitor was only introduced in .NET Framework 4.5.

[linkdotnet]

Okay now I understand a bit better - thanks for clarifying @MarkCiliaVincenti !

Now let's say you add a second method but want to have a timeout on it. Now you can neither use myObj.Enter(5) because on .NET 8.0 that won't work, nor can you use Monitor.Enter(myObj, 5) because on .NET 9.0 that has different behaviour.

True that - and here is what I meant initially. If we have such a situation, I totally agree, that some kind of backport is useful! The way bUnit is using locking is very simple and we don't have such cases. So my argument would still be: For now, I would postpone the usage of a micro-library for a case we don't have and use it once we hit this barrier.

Don't get me wrong: You did an amazing job, and lots of folks have such use cases, but bUnit doesn't (at the moment).
That said, @egil and I have our weekly sync/stream where we can discuss this. Everything you read so far is just my opinion.

[linkdotnet]

Furthermore, are there any other implications for <= net8.0 given that the semantics are different.

The only limitation is that on .NET Framework 3.5 and .NET Framework 4.0, you cannot use the IsHeldByCurrentThread property since the IsEntered() method on Monitor was only introduced in .NET Framework 4.5.

Fair enough. .NET FX 4.0 isn't supported anymore anyway. So targeting >netstandard2.0 and beyond is reasonable!

[MarkCiliaVincenti]

#if NET_9_0_OR_GREATER
private readonly Lock syncLock = new();
#else
private readonly object syncLock = new();
#endif

That is correct. It will work as per the code you wrote. There's also a neater way of doing it via:

<Using Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))"
		   Alias="Lock"
		   Include="System.Object" />

And if you do try to Monitor.Enter on it, it will still give you the warning CS9216: A value of type System.Threading.Lock converted to a different type will use likely unintended monitor-based locking in lock statement.

That said, I understand your argument with regards to crossing the bridge when you come to it, but it just feels a bit like procrastinating to me and I'd personally prefer to remove the limitation. If your biggest concern is adding in a new dependency, you may want to consider importing the class directly into your project, just giving some credit.

[linkdotnet]

That said, I understand your argument with regards to crossing the bridge when you come to it, but it just feels a bit like procrastinating to me and I'd personally prefer to remove the limitation.

It isn't about procrastinating or not - sorry if it felt that way. It is more about the choice we have to make to onboard something we have to maintain for years to come. Given that we support net8.0, we will support this at least until November 2026 (and as v1 shows we support even outdated versions).

We will add a dependency for users, so we have to think through it and not just rush into something. I do understand that you have an awesome library that solves specific issues for users.

And if you do try to Monitor.Enter on it, it will still give you the warning CS9216: A value of type System.Threading.Lock converted to a different type will use likely unintended monitor-based locking in lock statement.

I do understand and also agree that your library might solve that for many folks. My whole argumentation is based on the fact, that we don't have such code.

Given the pros and cons, what is the downside of delaying this to the time we are sure we will run into this situation? Or at least think of all the scenarios.

[linkdotnet]

@MarkCiliaVincenti

After discussing (link)the pros and cons, we decided that for bUnit that we keep the current object lock version even for .NET 9. If we use the new type, we will use the #ifdef NET_9_0_OR_GREATER approach as discussed earlier.

Thanks for filing the PR and discussing it!

[MarkCiliaVincenti]

@linkdotnet a new version of Backport.System.Threading.Lock has come out that acts as a source generator and basically dropping the DLL as a dependency. If this new development makes you reconsider, I will amend this PR accordingly.

[linkdotnet]

Hey @MarkCiliaVincenti - cool that you invested time into making a SG.

For me, nothing changes much here. It is somewhat in the same category as you proposed earlier:

[...] you may want to consider importing the class directly into your project, just giving some credit.

But thanks again for bringing up the topic with the updated library of yours!