Skip to content

Optimize IDisposable implementations #344

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

Conversation

SergeiPavlov
Copy link
Contributor

  • Make frequently invoked methods OpenSystemLogicOnlyRegion(), EnableSystemOperationRegistration(), DisableSystemOperationRegistration() alloc-less (there were 2 allocations: for Disposable<> & closure)
  • Optimize RegisterRoot()
  • Use static EmptyDisposable where possible
  • Refactor if (!hashSet.Contains(x)) hashSet.Add(x); pattern into hashSet.Add(x) to avoid double search
  • Fix PrefetchManagerBasicTest: prohibit JIT to inline the function

* Optimize IDisposable implementations

* optimize new List<>(1)

* Optimize EnableSystemOperationRegistration()

* Fix Race condition in tests

* Fix Race condition in tests

* Add [MethodImpl(MethodImplOptions.NoInlining)]
@@ -11,17 +11,26 @@ namespace Xtensive.Orm
{
public partial class Session
{
public readonly struct SystemLogicOnlyRegionScope : IDisposable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that we can make it internal and roll back to IDisposable in the DirectSessionAccessor.

Do you have any usage of DirectSessionAccessor.OpenSystemLogicOnlyRegion method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any usage of DirectSessionAccessor.OpenSystemLogicOnlyRegion method?

Yes, we do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is reasonable to restrict access to the scope constructor to keep creations within project

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roll back to IDisposable in the DirectSessionAccessor.

returning IDisposable requires heap allocation. That was the reason of this PR to avoid unnecessary allocations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, this is why I no longer require you to make type internal. I'm talking about the type constructor, there is no reason for it to be used outside project.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed SystemLogicOnlyRegionScope() ctor to be internal

@alex-kulakov alex-kulakov merged commit 8c8d8b8 into DataObjects-NET:master Dec 26, 2023
@SergeiPavlov SergeiPavlov deleted the upstream/OpenSystemLogicOnlyRegion branch July 11, 2024 23:17
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