Uh oh!
There was an error while loading. Please reload this page.
Optimize Array.GetFlattenedIndex (all runtimes) and InternalGetValue (CoreCLR) - #69940
Conversation
ghost
commented
May 28, 2022
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
5cdcf38 to
19c3aaeCompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
f52d283 to
60e1049Compare180dfae to
45cfa73Compareghost
commented
May 29, 2022
Tagging subscribers to this area: @dotnet/area-system-runtime Issue DetailsThis is only used when the element type is a reference type. EDIT: seems like the logic for
|
1ecf6b8 to
4c08cbcCompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
c113711 to
d88bd9eCompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
f8cced6 to
575b115CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Sergio0694
commented
May 30, 2022
Sure! Currently reading docs to get started with the performance repo, will share some numbers as soon as I have them 🙂 |
Benchmark results: | Method | Job | Toolchain | Mean | Error | StdDev | Ratio | RatioSD | Code Size ||---------------- |----------- |---------------------------- |----------:|----------:|----------:|------:|--------:|----------:|| GetValue_Object | Job-NBCGUL | \Core_Root\corerun.exe | 1.295 ns | 0.0818 ns | 0.1035 ns | 0.23 | 0.02 | 188 B || GetValue_Object | Job-IEBNQH | \Core_Root_base\corerun.exe | 5.748 ns | 0.1714 ns | 0.2669 ns | 1.00 | 0.00 | 231 B |||||||||||| GetValueInt | Job-NBCGUL | \Core_Root\corerun.exe | 44.603 ns | 0.8945 ns | 0.8367 ns | 0.96 | 0.02 | 188 B || GetValueInt | Job-IEBNQH | \Core_Root_base\corerun.exe | 46.517 ns | 0.5497 ns | 0.5142 ns | 1.00 | 0.00 | 231 B |For: [GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)]publicclassArrayGetValueBenchmark{privatereadonlyArrayarrayOfStrings;privatereadonlyArrayarrayOfInts;publicArrayGetValueBenchmark(){arrayOfStrings=new[]{"Hello","World","Bob","Alice","Fred","Tanner","Egor","Single"};arrayOfInts=Enumerable.Range(0,20).ToArray();}[Benchmark][BenchmarkCategory("REF_TYPE")]publicobject?GetValue_Object(){returnarrayOfStrings.GetValue(4);}[Benchmark][BenchmarkCategory("VALUE_TYPE")]publicobject?GetValueInt(){returnarrayOfInts.GetValue(4);}}Using the benchmark script kindly provided by @EgorBohere. Results seem in line with what was expected:
|
b29f1af to
424f829CompareThis is only used when the element type is a reference type
This is used whenever the element type is a class
* Move FCIMPL to comutilnative.cpp * Remove ExplicitLayout from TypeHandle * Tweak comment on unsupported path for Array.GetValue * Use MemoryMarshal.GetArrayDataReference instead of manual addressing * Add HELPER_METHOD_FRAME to FCall implementation
424f829 to
f98eb82Compare
This is only used when the element type is a reference type.
Idea came up while chatting with @jakobbotsch on Discord about array covariance being bad 😛
Also cc. @GrabYourPitchforks as this seems like his area as well.
I'm guessing we could also add a managed fast path for
SetValueby usingCastHelpers, but haven't done that yet.Do we want to explore that in this same PR or should I publish this and then we can eventually do that separately?
EDIT: seems like the logic for
SetValueis (at least in part) inCastHelpers.StelemRef, but that API is... Private? 🤔Not sure I understand how those are meant to be used, I'm sure there's a reason why they're all private...
API additions (CoreCLR-only)