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

Make use of System.Threading.Lock #20530

Open
wants to merge 26 commits into
base: dev
Choose a base branch
from

Conversation

MarkCiliaVincenti
Copy link
Contributor

.NET 9.0 comes with support for a new class System.Threading.Lock which offers better performance when locking as opposed to a lock on an object.

Through the use of a new dependency, System.Threading.Lock is backported to .NET 8.0 and earlier. On .NET 8.0 or earlier, there is neither a performance benefit nor a performance loss when using System.Threading.Lock. However, once ABP starts supporting .NET 9.0 or later, the code that's written will automatically benefit from improved performance.

@maliming maliming added this to the 9.1-preview milestone Aug 16, 2024
@maliming maliming self-requested a review August 16, 2024 01:00
@MarkCiliaVincenti
Copy link
Contributor Author

I asked a question on whether ABP wants to continue supporting .NET Standard 2.0 and 2.1 at #20803 (comment). The reason is that if this PR is going to get approved it would need to undergo changes. We would need to use the factory method in https://github.com/MarkCiliaVincenti/Backport.System.Threading.Lock rather than the clean method that we could use if ABP supports only .NET 5.0 or greater (basically anything that doesn't support the notorious thread aborts).

@maliming maliming modified the milestones: 9.0-preview, 9.0-final Oct 8, 2024
@MarkCiliaVincenti
Copy link
Contributor Author

@maliming do you still want to support .NET Standard 2.0 and 2.1? I want to be able to change this PR accordingly. It would be cleaner as an implementation of only .NET 5.0+ is supported. Due to thread aborts, there are issues with how synchronization can sometimes be broken in < 5.0, for example there is no 100% guarantee that anything that makes use of the using pattern is correctly disposed every time.

@MarkCiliaVincenti
Copy link
Contributor Author

@maliming due to assembly-level attributes, the only way this can avoid a dependency is that a new project file is created and everything in the library is copy and pasted into it, but it would be much cleaner with a dependency that's also tried and tested on production projects.

@MarkCiliaVincenti
Copy link
Contributor Author

@maliming do you still want to support .NET Standard 2.0 and 2.1? I want to be able to change this PR accordingly. It would be cleaner as an implementation of only .NET 5.0+ is supported. Due to thread aborts, there are issues with how synchronization can sometimes be broken in < 5.0, for example there is no 100% guarantee that anything that makes use of the using pattern is correctly disposed every time.

Still need an answer for this @maliming.

@maliming
Copy link
Member

hi

We will continue to support .NET Standard 2.0 and 2.1

#2668 (comment)

@MarkCiliaVincenti MarkCiliaVincenti changed the title Encourage use of System.Threading.Lock Make use of System.Threading.Lock Oct 15, 2024
@MarkCiliaVincenti
Copy link
Contributor Author

@maliming 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 interests you, I will amend this PR accordingly.

@maliming
Copy link
Member

@ebicoglu What do you think?

@MarkCiliaVincenti
Copy link
Contributor Author

Changes applied.

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 this pull request may close these issues.

2 participants