-
Notifications
You must be signed in to change notification settings - Fork 256
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
Port Port System.Linq.Performance.Tests #90
Conversation
|
note: some benchmarks have argument called |
=> Perf_LinqTestBase.Measure(_sizeToPreallocatedArray[size], iteration, wrapType, col => col.Where(o => o >= 0).Select(o => o + 1)); | ||
|
||
[Benchmark] | ||
[ArgumentsSource(nameof(IterationSizeWrapperData))] // for some reason the size and iteration arguments are ignored for this benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// for some reason the size and iteration arguments are ignored for this benchmark [](start = 60, length = 82)
Is this a bug?
} | ||
|
||
[Benchmark] | ||
[ArgumentsSource(nameof(IterationSizeWrapperData))] // for some reason the size and iteration arguments are ignored for this benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// for some reason the size and iteration arguments are ignored for this benchmark [](start = 60, length = 82)
Is this a bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will use GIT blame and contact the author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not new bugs, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not new bugs, right?
Not new, I just decided to add the comment because it was weird and I wanted to make it clear why the benchmarks are not using argument values, but instead some hardcoded values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should open an issue to track this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public const string V8 = "V8"; | ||
public const string Perflab = "Perflab"; | ||
|
||
public const string CoreFX = "CoreFX"; | ||
|
||
public const string LINQ = "LINQ"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public const string LINQ = "LINQ"; [](start = 8, length = 34)
Are there duplicated benchmarks here? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there was no duplication between CoreCLR and CoreFX tests. Both benchmarks test different LINQ parts. there are some common parts but:
CoreCLR LINQ test use complex types for tests, CoreFX mostly just ints
CoreCLR LINQ test iterate over the results, CoreFX call .ToArray() (a bad thing)
So I did not remove any benchmarks, just added them to a LINQ category
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Thanks! :) #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #57
78 benchmarks, BDN needed 9 minutes, xunit 2 minutes. The huge difference is due to the fact that BDN run more iterations.
BDN produced more stable and accurate results.
These benchmarks need a redesign, for almost every single tested thing
.ToArray()
is called at the end. Which is a huge factor and can be a dominating cost. Those benchmarks should do sth similar to CoreCLR LINQ benchmarks: just iterate over theIEnumberable
@jorive there was no duplication between CoreCLR and CoreFX tests. Both benchmarks test different LINQ parts. there are some common parts but:
.ToArray()
(a bad thing)So I did not remove any benchmarks, just added them to a
LINQ
category