Add a few more benchmarks#1449
Conversation
angularsen
left a comment
There was a problem hiding this comment.
Wow! A lot of great work here. It's more or less ready to merge, just one comment.
| public TemperatureDelta SumOfDeltas() | ||
| { | ||
| #if NET | ||
| return UnitsNet.GenericMath.GenericMathExtensions.Sum(_quantities); |
There was a problem hiding this comment.
I think using GenericMathExtension for net50+ and regular addition for net48 is an apples to oranges comparison, ref the #if NET condition.
I see that all the "Sum" tests involve GenericMathExtensions, but I think we should have dedicated tests for that class vs performance tests of the addition operators alone, to get meaningful results.
There was a problem hiding this comment.
In my "Fraction" based implementation I've got the same extension prototype applicable to both types- so I'm comparing NET Framework v6 to NET Framework v6 with the new extension.
By itself the test only shows how much faster (ratio) the NET is compared to NET Framework when adding a bunch of quantities (also- the scaling doesn't linear with the fractions).
|
Regarding the github action, the results back then were too variable to be useful. The benchmarks are fine, but we can't rely on github agents to do any consistent testing I think, unless they have come up with any special agents dedicated for performance testingd. |
|
I was thinking of introducing the I'm still lacking any dedicated benchmarks for the However there are still a lot more optimizations than just the use of the |
|
This all sounds good, I'm fine adding the |
In order to better compare the performance between the existing v6 and the one with the Fractions I've been using these benchmarks. They are not very polished (and I haven't had the time for the string-related stuff) but they do the trick.
You don't have to merge them if you don't want to, but at least they are here (so I can reference the code).
I'll probably add the results from my local runs in a comment (or more like a bunch of comments) later on (my previous results got wiped by the build script 😆) - but I was hoping that we could update the packages and add net9.0 to the list of targets first (there are lots of a cool stuff in net9).
PS I'm not sure if that old
gh-actionwould still work or not, but even adding the results by hand (here?) is an option.Here's what I did for the Fractions (the
Readme.mdwas 90% AI generated).