Moving fast parts of cast FCALLs to managed code. #1068

VSadov · 2019-12-19T21:05:10Z

Exposed casting cache to managed code
Implemented a managed version of cache lookup
Moved JIT_IsInstanceOfAny and JIT_ChkCastAny to managed code as the first ones to move.
Skip managed JIT helpers in exception stack traces and debugger
Managed JIT_IsInstanceOfInterface
All other cast helpers are managed.

Fixes:https://github.com/dotnet/coreclr/issues/27931

andrew-boyarshin · 2019-12-29T06:05:28Z

src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs

+        }
+
+        // NOTE!!
+        // This is a copy of C++ implementation in CastCache.cpp


More likely castcache.h. #Resolved

Nit: The implementation is in the .cpp file, but the VM filename is in lower case (castcache.cpp). #Resolved

VSadov · 2019-12-30T19:43:00Z

@tommcdon - tried the "logical unwind" here in the last change. Does not seem to be sufficient.
VS still visualizes invalid casts as rethrows. #Resolved

tommcdon · 2020-01-01T01:06:29Z

@tommcdon - tried the "logical unwind" here in the last change. Does not seem to be sufficient.
VS still visualizes invalid casts as rethrows.

Proposed fix at a lower level is here: VSadov@d269b62. Tested the fix in VS and it seems to work correctly. #Resolved

VSadov · 2020-01-19T02:06:09Z

I think this is ready to be reviewed. #Resolved

VSadov · 2020-01-19T02:08:09Z

The change moves fast parts of casting helpers to managed code and removes various existing implementations - in C++, in asm in C/inlined asm. Now it is all C#. #Resolved

VSadov · 2020-01-19T02:21:09Z

I also hoped to not have specialized versions for class and interface casts.

Tuns out outperforming hand-tuned assembly is challenging, so specialized methods are still there.
I think cache lookup must get 20-30% faster before considering just using cache. As it is, best case scenarios for classes and interfaces are noticeably faster than cache lookups and these scenarios also seem fairly common.

Whoever wrote the AMD64 assembly helpers did a god job.

I have discussed with @davidwrighton possible ways of speeding up the cache lookup:

use singleton table instead of null for cache flush (will eliminate one null check)
use fixed size table (will simplify reducing hashcode to table index)
consider simpler hash function, perhaps at cost of larger table.
inline cache lookup by hand (some codepaths could be streamlined)
precompute hashed value of methodtable pointer.
. . . something else..

This may improve the cache lookup enough to switch Interface and/or Class case to Any or maybe it will not.

I think perf tinkering should be a separate change though.
This PR achieves its goals - the helpers are managed and perf is roughly the same or better. #Resolved

jkotas · 2020-01-19T02:22:44Z

src/coreclr/src/inc/corinfo.h

@@ -423,12 +423,10 @@ enum CorInfoHelpFunc
    // the right helper to use

    CORINFO_HELP_ISINSTANCEOFINTERFACE, // Optimized helper for interfaces
-    CORINFO_HELP_ISINSTANCEOFARRAY,  // Optimized helper for arrays


I would keep these for now, even if it means that the implementation is same. #Resolved

I can get them back.

In reply to: 368260447 [](ancestors = 368260447)

I just could not see what could be done differently in a helper if we knew we are casting to an array.
For identity cast check we could now just compare MTs, which we do for any cast...

In reply to: 368261248 [](ancestors = 368261248,368260447)

I am guessing JIT could inline identity cast check similarly to class cast. Not sure if those are common with arrays but with classes inlining identity cast helps a lot.

In reply to: 368261350 [](ancestors = 368261350,368261248,368260447)

VSadov · 2020-01-19T02:24:48Z

Here is what I see on a microbenchmark that calls in a loop a method that performs a particular kind of cast


Empty: 232ms      <-- overhead, should be _roughly_ subtracted from other measurements.
Base to Base: 417ms    <-- inlined by the JIT, nearly free
Derived to Base: 504ms    <-- IsInstanceOfClassSpecial  1 iteration
Derived2 to Base: 552ms   <-- IsInstanceOfClassSpecial  3 iterations
Derived4 to Base: 622ms   <-- IsInstanceOfClassSpecial  5 iterations
Derived6 to Base: 754ms   <-- IsInstanceOfClassSpecial  7 iterations. no-nullcheck IsInstanceAny would have similar cost. 

List<int> to IList<int>: 601ms       <-- IsInstanceOfInterface 1 iteration
List<int> to ICollection<int>: 644ms <-- IsInstanceOfInterface 2 iterations
List<int> to IEnumerable<int>: 861ms <-- IsInstanceAny   (3rd, but JIT does not like variant interfaces)
List<int> to ICollection: 589ms      <-- IsInstanceOfInterface 5 iterations
List<double> to ICollection<int>: 719ms   <-- IsInstanceOfInterface 8 iterations
Dictionary<double, double> to IDeserializationCallback: 859ms   <-- IsInstanceOfInterface 10 iterations.

List<int> to IReadOnlyCollection<int>: 1062ms  <-- IsInstanceAny, looks like a hash collision
List<string> to IReadOnlyCollection<object>: 842ms  <-- IsInstanceAny  pass
List<double> to IReadOnlyCollection<int>: 966ms     <-- IsInstanceAny  failure 
string[] to IReadOnlyCollection<object>: 858ms      <-- IsInstanceAny  pass

where the baseline is

Empty: 352ms	<-- overhead, no idea why slower than Base to Base. branch misprediction?
Base to Base: 325ms
Derived to Base: 627ms
Derived2 to Base: 599ms
Derived4 to Base: 691ms
Derived6 to Base: 789ms

List<int> to IList<int>: 550ms
List<int> to ICollection<int>: 724ms
List<int> to IEnumerable<int>: 1075ms  <-- new IsInstanceAny is a bit faster (due to cache improvements, I think).
List<int> to ICollection: 739ms
List<double> to ICollection<int>: 783ms
Dictionary<double, double> to IDeserializationCallback: 919ms

List<int> to IReadOnlyCollection<int>: 973ms
List<string> to IReadOnlyCollection<object>: 1365ms
List<double> to IReadOnlyCollection<int>: 1015ms
string[] to IReadOnlyCollection<object>: 971ms

#Resolved

...clr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

VSadov · 2020-01-19T02:39:04Z

The Linux is similar, but there are few interesting differences.

Since we did not use hand-written assembly on Linux and C++ helpers were not as sophisticated, there is some gain in specialized methods.

On the other hand the improvements in cache lookup seems to have been negated by something. We even see a 1-2% regression. I think it is acceptable, but wonder why we have it.

Either we have some platform specific penalty for moving to managed (call conventions, access of statics?), or GCC produces better code (compared to MSVC) for native cache lookup and JIT is not matching that.
It would be interesting to find out. Maybe we can improve something.

===== baseline

Empty: 338ms
Base to Base: 397ms
Derived to Base: 588ms
Derived2 to Base: 678ms
Derived4 to Base: 1141ms
Derived6 to Base: 1056ms
List<int> to IList<int>: 649ms
List<int> to ICollection<int>: 676ms
List<int> to IEnumerable<int>: 963ms
List<int> to ICollection: 1092ms
List<double> to ICollection<int>: 1302ms
Dictionary<double, double> to IDeserializationCallback: 1264ms
List<int> to IReadOnlyCollection<int>: 983ms
List<string> to IReadOnlyCollection<object>: 988ms
List<double> to IReadOnlyCollection<int>: 989ms
string[] to IReadOnlyCollection<object>: 991ms

=== after the change:

Empty: 286ms
Base to Base: 453ms
Derived to Base: 755ms
Derived2 to Base: 816ms
Derived4 to Base: 844ms
Derived6 to Base: 994ms
List<int> to IList<int>: 679ms
List<int> to ICollection<int>: 623ms
List<int> to IEnumerable<int>: 1051ms
List<int> to ICollection: 967ms
List<double> to ICollection<int>: 867ms
Dictionary<double, double> to IDeserializationCallback: 1164ms
List<int> to IReadOnlyCollection<int>: 1234ms
List<string> to IReadOnlyCollection<object>: 1166ms
List<double> to IReadOnlyCollection<int>: 1019ms
string[] to IReadOnlyCollection<object>: 1111ms
``` #Resolved

VSadov · 2020-01-19T02:41:53Z

Note - all the measurements have some degree of noise.
At these level we are sensitive to alignment of code blocks and jumps/labels, to branch mispredictions and the like. So sometimes results vary for seemingly no reasons. #Resolved

src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs

Also renamed `BaseMethodTable` to `ParentMethodTable` - to match native code closer.

Missed an attribute in one place

VSadov · 2020-01-22T18:53:44Z

Removed AggressiveOptimization from casting helpers.

VSadov · 2020-01-22T19:01:18Z

Thanks!!

benaadams · 2020-02-02T17:48:16Z

Does build --subsetCategory coreclr -configuration release R2R the System.Private.CoreLib.dll (think it does as it also produces a System.Private.CoreLib.ni.pdb)

If so can these be added to R2R? Am seeing ChkCastAny as showing up as Jitting during startup:

jkotas · 2020-02-03T04:26:10Z

I do not see ChkCastAny JITed during startup. For the record, here is the list of methods that I see getting JITed during startup (CoreLib R2R, empty Main):

System.SpanHelpers:IndexOf(byref,ushort,int):int
System.Runtime.Intrinsics.Vector128`1[UInt16][System.UInt16]:get_Count():int
Internal.Runtime.CompilerServices.Unsafe:Add(byref,long):byref
System.Runtime.Intrinsics.Vector128:CreateScalarUnsafe(ushort):System.Runtime.Intrinsics.Vector128`1[UInt16]
System.Runtime.Intrinsics.Vector128:AsUInt32(System.Runtime.Intrinsics.Vector128`1[UInt16]):System.Runtime.Intrinsics.Vector128`1[UInt32]
System.Runtime.Intrinsics.Vector128:AsUInt16(System.Runtime.Intrinsics.Vector128`1[UInt32]):System.Runtime.Intrinsics.Vector128`1[UInt16]
System.Runtime.Intrinsics.Vector256`1[UInt16][System.UInt16]:get_Count():int
System.Text.Unicode.Utf16Utility:GetPointerToFirstInvalidChar(long,int,byref,byref):long
System.Runtime.Intrinsics.Vector128:CreateScalarUnsafe(short):System.Runtime.Intrinsics.Vector128`1[Int16]
System.Runtime.Intrinsics.Vector128:AsInt32(System.Runtime.Intrinsics.Vector128`1[Int16]):System.Runtime.Intrinsics.Vector128`1[Int32]
System.Runtime.Intrinsics.Vector128:AsInt16(System.Runtime.Intrinsics.Vector128`1[Int32]):System.Runtime.Intrinsics.Vector128`1[Int16]
System.Runtime.Intrinsics.X86.Sse41:get_IsSupported():bool
System.Runtime.Intrinsics.Vector128:AsUInt16(System.Runtime.Intrinsics.Vector128`1[Int16]):System.Runtime.Intrinsics.Vector128`1[UInt16]
System.Runtime.Intrinsics.Vector128:AsByte(System.Runtime.Intrinsics.Vector128`1[UInt16]):System.Runtime.Intrinsics.Vector128`1[Byte]
System.Text.ASCIIUtility:NarrowUtf16ToAscii(long,long,long):long
X64:get_IsSupported():bool
X64:ParallelBitExtract(long,long):long

What is the list of methods that you see JITed during startup?

benaadams · 2020-02-03T04:55:10Z

What is the list of methods that you see JITed during startup?

Maybe is the Profiler causing recompilation then?

xiangzhai · 2020-02-03T07:51:32Z

:mips-interest

VSadov force-pushed the castFcalls branch 2 times, most recently from 63cb4c5 to 9ca0b1b Compare December 21, 2019 21:27

jkotas added the area-VM-coreclr label Dec 27, 2019

VSadov force-pushed the castFcalls branch from 9ca0b1b to 20f7a30 Compare December 28, 2019 05:46

andrew-boyarshin reviewed Dec 29, 2019

View reviewed changes

VSadov force-pushed the castFcalls branch 6 times, most recently from 0d03012 to f0a5986 Compare January 11, 2020 03:05

VSadov force-pushed the castFcalls branch 2 times, most recently from ff792a3 to 0729321 Compare January 13, 2020 07:47

VSadov mentioned this pull request Jan 16, 2020

Improvements for null check folding. #1735

Merged

VSadov force-pushed the castFcalls branch 2 times, most recently from 5d743cc to a0223a9 Compare January 18, 2020 21:03

VSadov changed the title ~~[WIP] Moving fast parts of cast FCALLs to managed code.~~ Moving fast parts of cast FCALLs to managed code. Jan 19, 2020

VSadov requested review from jkotas and davidwrighton January 19, 2020 02:05

jkotas reviewed Jan 19, 2020

View reviewed changes

...clr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs Outdated Show resolved Hide resolved

jkotas reviewed Jan 19, 2020

View reviewed changes

src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs Outdated Show resolved Hide resolved

VSadov added 18 commits January 21, 2020 23:22

Managed JIT_IsInstanceOfInterface

697f531

deleted the array casting helpers. not special enough now

7697ee6

reconcilled changes between managed and native versions of TryGet

7ec7a3b

managed JIT_IsInstanceOfClass

8587cae

remaining casting helpeers are managed

c5e59e3

move helpers init to ECall.cpp

a4a46f6

some perf tweaks

508ca1a

Cleanup for review

ad6054c

Add AggressiveOptimization

71b0743

Reverted removal of array helpers. They are mapped to "any" for now.

a2d6356

Removed JITUtil_ and JIT_ from managed helpers

4c56bfc

Turn BaseMethodTable and InterfaceMap into fields.

bf46b11

Comments about nuances of InterfaceMap and ParentMethodTable.

4f7bb17

Also renamed `BaseMethodTable` to `ParentMethodTable` - to match native code closer.

check exact number of interfaces (do not assume 4-items alignment)

b62ac70

MethodTable** InterfaceMap.

ccd60ac

Missed an attribute in one place

comment

32f83f3

GC poll in 2 remaining cast FCALLs

e5e9d80

Removed AggressiveOptimization from casting helpers.

fab2e97

VSadov force-pushed the castFcalls branch from 3422e44 to fab2e97 Compare January 22, 2020 07:23

jkotas approved these changes Jan 22, 2020

View reviewed changes

VSadov merged commit c3dc1fd into dotnet:master Jan 22, 2020

VSadov mentioned this pull request Mar 5, 2020

IReadOnlyCollection<T> does not implement ICollection<T> #33245

Closed

ghost locked as resolved and limited conversation to collaborators Dec 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving fast parts of cast FCALLs to managed code. #1068

Moving fast parts of cast FCALLs to managed code. #1068

VSadov commented Dec 19, 2019 •

edited

Loading

andrew-boyarshin Dec 29, 2019 •

edited by VSadov

Loading

jkotas Jan 19, 2020 •

edited by VSadov

Loading

VSadov commented Dec 30, 2019 •

edited

Loading

tommcdon commented Jan 1, 2020 •

edited by VSadov

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

jkotas Jan 19, 2020 •

edited by VSadov

Loading

VSadov Jan 19, 2020

VSadov Jan 19, 2020 •

edited

Loading

VSadov Jan 19, 2020

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 22, 2020

VSadov commented Jan 22, 2020

benaadams commented Feb 2, 2020

jkotas commented Feb 3, 2020

benaadams commented Feb 3, 2020

xiangzhai commented Feb 3, 2020

Moving fast parts of cast FCALLs to managed code. #1068

Moving fast parts of cast FCALLs to managed code. #1068

Conversation

VSadov commented Dec 19, 2019 • edited Loading

andrew-boyarshin Dec 29, 2019 • edited by VSadov Loading

Choose a reason for hiding this comment

jkotas Jan 19, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

VSadov commented Dec 30, 2019 • edited Loading

tommcdon commented Jan 1, 2020 • edited by VSadov Loading

VSadov commented Jan 19, 2020 • edited Loading

VSadov commented Jan 19, 2020 • edited Loading

VSadov commented Jan 19, 2020 • edited Loading

jkotas Jan 19, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

VSadov Jan 19, 2020

Choose a reason for hiding this comment

VSadov Jan 19, 2020 • edited Loading

Choose a reason for hiding this comment

VSadov Jan 19, 2020

Choose a reason for hiding this comment

VSadov commented Jan 19, 2020 • edited Loading

VSadov commented Jan 19, 2020 • edited Loading

VSadov commented Jan 19, 2020 • edited Loading

VSadov commented Jan 22, 2020

VSadov commented Jan 22, 2020

benaadams commented Feb 2, 2020

jkotas commented Feb 3, 2020

benaadams commented Feb 3, 2020

xiangzhai commented Feb 3, 2020

VSadov commented Dec 19, 2019 •

edited

Loading

andrew-boyarshin Dec 29, 2019 •

edited by VSadov

Loading

jkotas Jan 19, 2020 •

edited by VSadov

Loading

VSadov commented Dec 30, 2019 •

edited

Loading

tommcdon commented Jan 1, 2020 •

edited by VSadov

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

jkotas Jan 19, 2020 •

edited by VSadov

Loading

VSadov Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading

VSadov commented Jan 19, 2020 •

edited

Loading