[Perf -16%] System.Collections.CopyTo<String> (6) #37814
Comments
DrewScoggins
added
the
tenet-performance-benchmarks
Dotnet-GitSync-Bot
added
area-System.Collections
untriaged
|
Tagging subscribers to this area: @eiriktsarpalis |
eiriktsarpalis
removed
the
untriaged
DrewScoggins
added
tenet-performance
|
Commit range as best I can tell. @DrewScoggins the interactive graph will not set a commit for "before" for the runtime repo -- it just says "0". So I bounded the start by the date/time of that datapoint. Is that right, or am I reading it wrong? Nothing jumps out there. I guess next step is to profile the scenario for 5.0 vs 3.1 all up. This would also eliminate a configuration/microcode change. @DrewScoggins do we have any way eg more datapoints to bound this better? Also, how can I see the full trend from 3.1 to now? |
|
Reproduces locally. Note that ImmutableArray scenario didn't regress, presumably because nothing is actually copied 😄 BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.421 (2004/?/20H1) PowerPlanMode=00000000-0000-0000-0000-000000000000 Arguments=/p:DebugType=portable IterationTime=250.0000 ms
|
|
Note the Array.Copy regression was in November (#39112) *Memory, *Span use Buffer.Memmove @jkotas thoughts about this one, since we were discussing earlier Array.Copy change? Also, why is it OK for Span to always use Memmove? |
|
Also #37808 is about a regression (on Ubuntu) for 1000 element |
Span uses generic version of Memmove that calls non-generic Memmove or BulkMoveWithWriteBarrier for CoreCLR. The logic is same for both Span and Array: |
Memory copying performance depends a lot on alignment of the source and destination arrays. In the case of object references that we have here, it also depends on the generation that the destination array lives in. I do not see the benchmarks controlling for these variables. It makes the benchmark many modal by construction. Notice that the trend changed in January, but it has several flips to baseline in February. It confirms the hypothesis. |
|
Here is a small program that demonstrates the many modal nature of memory copying: On my machine, it shows that the performance is oscilating between ~460 and ~800. |
|
What do the |
|
|
|
OK thanks I think next action here for us is to make changes like that in the benchmarks and remeasure 3.1 and 5.0 |
|
I agree. This looks like a place where the test is not measuring the right thing. Also to answer your questions Dan: These 3.1->5.0 comparisons are weird because they span the coreclr/corefx->runtime switch. As a result I never really built in a way to do commit level comparisons across the repos, but I would say that what you did was correct. It would be hard to go bak and get any more data points to try and bound this, but it would be possible. Going forward we should not have this problem as we have more reliable ways of triggering runs out the installer repo, and running perf tests in the runtime repo directly. |
Nice "trick"! I wonder if BenchmarkDotNet should do something like this between the iterations to try to ensure more random memory alignments (cc @AndreyAkinshin @billwert ) |
Would be an interesting experiment! Perhaps we should take some random sample of benchmarks (1/type?) and see what stability this gives us over repeated runs compared to not doing it? |
|
Does BDN have an opportunity to do this? In this case it needs to influence the alignment of arrays that are allocated within the setup method: [GlobalSetup]
public void Setup()
{
_array = ValuesGenerator.ArrayOfUniqueValues<T>(Size);
_list = new List<T>(_array);
_immutablearray = Immutable.ImmutableArray.CreateRange(_array);
_destination = new T[Size];
}If I'm understanding correctly, the |
|
@layomia @eiriktsarpalis Should we move this to 6.0.0? |
|
@jeffhandley Based on the comments above I think it makes sense to move. |
|
We might need to check if the regression remains after dotnet/BenchmarkDotNet#1513 got addressed. I tried finding more recent benchmark results for the particular benchmark but I can't seem to be able to navigate to that via the reporting system. |
|
This issue has been automatically marked |
|
@eiriktsarpalis If you ever want to look at historical data, the best way to do that is to go to https://aka.ms/HistoryIndex. From there you can click on the link to the configuration you want to look at, and the search in the index page for the test of interest. |
ghost
added
needs-further-triage
|
Thanks @DrewScoggins, I was not aware of that link. Is it documented somewhere, or is it possible to include as a link in regression reports? |
|
Assuming the benchmark hardware is the same, it would seem like the regressions have been resolved. That being said, the List case seems to be displaying frequent spikes in performance since July 2021: It would seem to me though that this is unrelated to the root cause that prompting the filing of this original issue, so wondering if we should just close it? |
|
The
Which gives us 3 modes visible in the charts: all are aligned, only one is aligned, none are aligned. This is why I've used it as an example when introducing the Memory Randomization feature to BDN.
I agree. 👍 |
It has not been documented as of yet, as it is kind of hacky :) However, I can start putting that link into the reports, with a little description of how to use it. |
ghost
locked as resolved and limited conversation to collaborators
Run Information
Regressions in System.Collections.CopyTo
Historical Data in Reporting System
Repro
Histogram
System.Collections.CopyTo.Span(Size: 2048)
System.Collections.CopyTo.Memory(Size: 2048)
System.Collections.CopyTo.Array(Size: 2048)
System.Collections.CopyTo.ReadOnlySpan(Size: 2048)
System.Collections.CopyTo.ReadOnlyMemory(Size: 2048)
System.Collections.CopyTo.List(Size: 2048)
Docs
Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository
The text was updated successfully, but these errors were encountered: