Skip to content

Conversation

@tool-buddy
Copy link
Contributor

Hi
In a nutshell: replaced "new T()" with a more optimized method, based on this: https://stackoverflow.com/a/29972767
Because of performance deteriorating introduced in between version 1.0.55 and 1.0.64, I didn't upgrade to 1.0.64, thus the optimization is based on 1.0.55. Since that version, other "new T()"s were introduced in the code base, which can benefit from the same optimization introduced by this pull request.
Thanks for your time

@speps
Copy link
Owner

speps commented Mar 14, 2021

I don't understand the optimisation, in every case new performed better. It just says a compiled expression is better than Activator.CreateInstance. Am I reading it wrong?

@tool-buddy
Copy link
Contributor Author

What you are reading wrong is that the the "Y" of the "new Y()" is a defined type, but in the modified code the "T" of the "new T()" is not a defined type. In that case, the "new T()" expression leads to a call to Activator.CreateInstance. That's what I saw when I was profiling the code. You can run some code that creates a lot of new T()s, the difference in performance will be quite visible between the two versions.

@speps
Copy link
Owner

speps commented Mar 16, 2021

Thanks @ehakram, I found another article which details a lot more than the SO answer. I tried a few different ones, and they all seem similar, about 30% faster. So I'll go with yours.

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.746 (2004/?/20H1)
Intel Core i7-10850H CPU 2.70GHz, 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=5.0.103
  [Host]     : .NET Core 3.1.12 (CoreCLR 4.700.21.6504, CoreFX 4.700.21.6905), X64 RyuJIT  [AttachedDebugger]
  DefaultJob : .NET Core 3.1.12 (CoreCLR 4.700.21.6504, CoreFX 4.700.21.6905), X64 RyuJIT


|   Method |      Mean |    Error |   StdDev |
|--------- |----------:|---------:|---------:|
|  Default | 101.17 ns | 1.974 ns | 3.188 ns |
|     PR46 |  70.75 ns | 1.438 ns | 1.346 ns |
| Improved |  72.04 ns | 1.418 ns | 1.688 ns |
|  Dynamic |  70.28 ns | 1.136 ns | 1.352 ns |

@speps speps merged commit c113505 into speps:master Mar 16, 2021
@tool-buddy
Copy link
Contributor Author

Thanks a lot, and thanks for making LibTessDotNet in the first place.
Like said in the original message, my branch optimized the 1.0.5.5 code, there has been other "new T()"s introduced since. And if you have any information about the performance decrease I witnessed between 1.0.5.5 and 1.0.64, please let me know.
Have a nice day

@speps
Copy link
Owner

speps commented Mar 17, 2021

In regards to your question, I compared 1.0.55 and 1.0.64 and couldn't find anything that jumped at me in regards to the performance degradation you've noticed. Hopefully the latest v1.1.15 can recover this performance as it pools some more internal types now.

@tool-buddy
Copy link
Contributor Author

1.1.15 didn't improve performance back to 1.0.55 level in my tests. I will open a proper issue with data and test code sometime in the (probably not so near) future.

@speps speps mentioned this pull request Apr 12, 2022
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