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

Replace ConstructorInfo Invoke with CreateInstance for improved performance #646

Conversation

bethmaloney
Copy link
Contributor

Reflection is used in the PersistenceModel to initialize the mapping classes. Currently we're using the ConstructorInfo.Activate function to create the mapping objects. Activator.CreateInstance is a bit more than twice as fast and is less code.

Unfortunately, there's a fair bit of work happening in the mapping constructors so we don't see such a dramatic improvement in initialization time. Benchmarking the change against actual mapping classes and the initialization time drops by a few percentage points depending on the complexity of class.

Benchmarking the construction time of the ProductMap class in the example project

| Method            | Mean     | Error    | StdDev   |
|------------------ |---------:|---------:|---------:|
| ConstructorInfo   | 52.47 us | 1.042 us | 1.070 us |
| ActivatorInstance | 50.50 us | 0.628 us | 0.557 us |

Not a huge performance increase but it's an easy change to make.

@bethmaloney bethmaloney changed the title Replace Constructor Info Invoke with CreateInstance For Improved Performance Replace ConstructorInfo Invoke with CreateInstance for improved performance Feb 29, 2024
@bethmaloney
Copy link
Contributor Author

@hazzik Sorry for pinging you but the PR has been open for over a month now. Is there something I can do to make this PR easier to review?

@hazzik hazzik merged commit 9675ab5 into nhibernate:main Apr 2, 2024
3 checks passed
@hazzik hazzik added this to the v3.4.0 milestone Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants