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

Locking performance optimizations for .NET 9.0+ #956

Closed

Conversation

MarkCiliaVincenti
Copy link

@MarkCiliaVincenti MarkCiliaVincenti commented Aug 13, 2024

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Summary of the changes
Targeting of .NET 9.0 and locking performance optimizations for .NET 9.0+

Fixes #955

Description

Targeting of .NET 9.0 and locking performance optimizations for .NET 9.0+. Backported to all prior versions of .NET without performance degradation.

@MarkCiliaVincenti
Copy link
Author

Would need to be rewritten to harden against thread aborts. Let me know if interested.

@rclabo
Copy link
Contributor

rclabo commented Oct 1, 2024

Related Reddit post by @MarkCiliaVincenti which provides a nice background summary.

While in the longer term, this is, of course, very interesting, I can't see it being much of a priority given that there is no performance gain unless running on .NET 9. Just my thoughts.

@MarkCiliaVincenti
Copy link
Author

Related Reddit post by @MarkCiliaVincenti which provides a nice background summary.

While in the longer term, this is, of course, very interesting, I can't see it being much of a priority given that there is no performance gain unless running on .NET 9. Just my thoughts.

That is correct. The performance gain is only on .NET 9 or greater, but there is no performance loss for .NET 8 or earlier.

More and more people will start adopting .NET 9, and later .NET 10, at which point the assumption is that .NET 8 would still be targeted by Lucene.NET. Therefore it stands to reason that one should ask whether or not it should be done. If the answer is yes, delaying it provides no value, except sensible ones such as waiting for .NET 9.0 to be officially released.

@paulirwin
Copy link
Contributor

@MarkCiliaVincenti Thank you for this PR, but there are a few reasons why we are going to close this at this time. First, as you might have noticed, it has been quite some time since our last release. At this time, to prevent having to continually shift our targets, we are not planning on targeting .NET 9 for this next release. (Additionally, if we were going to consider that, we would want that change done separate from locking changes.) We also have just gone through fixing a number of concurrency bugs for this release, and are hesitant to make any changes at this time. We will keep the issue open and reconsider this for a future release. Thanks again!

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.

Make use of .NET 9.0's System.Threading.Lock which is more performant
3 participants