mono_magic_trampoline being called more than expected #17334
Assignees
Labels
Projects
Comments
fanyang-mono
added
task
area-Runtime: Performance
lambdageek
added a commit
to lambdageek/mono
that referenced
this issue
Oct 15, 2019
Exchange<T> is supposed to be picked up as an intrinsic. But if it isn't, since T has a class constraint, call the `Exchange (ref object, ref object, ref object)` overload instead. This works around a limitation in the JIT: https://github.com/mono/mono/blob/82a07273c22b996b08fa88ee2e6632d782ac2242/mono/mini/mini-trampolines.c#L704-L713 if we're in the common call trampoline, and the caller is generic method and the callee is a generic method, if we're using generic sharing for the caller, but there's no generic jit info for the callee, we will not patch the call site. In this case we had: Exchange<T> calls Exchange_T<T> where the callee is an icall (and evidently didn't have generic jit info). So every time we called Exchange<T>, we would go through the trampoline when trying to call Exchange_T<T> without patching teh call site. Addresses part of mono#17334
lambdageek
added a commit
to lambdageek/mono
that referenced
this issue
Oct 15, 2019
Exchange<T> is supposed to be picked up as an intrinsic. But if it isn't, since T has a class constraint, call the `Exchange (ref object, ref object, ref object)` overload instead. This works around a limitation in the JIT: https://github.com/mono/mono/blob/82a07273c22b996b08fa88ee2e6632d782ac2242/mono/mini/mini-trampolines.c#L704-L713 if we're in the common call trampoline, and the caller is generic method and the callee is a generic method, if we're using generic sharing for the caller, but there's no generic jit info for the callee, we will not patch the call site. In this case we had Exchange<T> calling Exchange_T<T> where the callee is an icall (and evidently didn't have generic jit info). So every time we called Exchange<T>, we would go through the trampoline when trying to call Exchange_T<T> without patching the call site. Addresses part of mono#17334
|
I added some logging to Or short, The runtime couldn't patch the caller, as explained and addressed in this PR: #17341 , so it would over and over end up in the magic trampoline. However, we should never need to patch it because its caller ( Closing it as a duplicate of #17335 |
Short Term Projects
automation
moved this from Backlog
to Completed Tasks in this Sprint
Oct 16, 2019
lambdageek
added a commit
that referenced
this issue
Oct 16, 2019
* [bcl][jit] implement Interlocked.Exchange<T> in terms of object Exchange<T> is supposed to be picked up as an intrinsic. But if it isn't, since T has a class constraint, call the `Exchange (ref object, ref object, ref object)` overload instead. This works around a limitation in the JIT: https://github.com/mono/mono/blob/82a07273c22b996b08fa88ee2e6632d782ac2242/mono/mini/mini-trampolines.c#L704-L713 if we're in the common call trampoline, and the caller is generic method and the callee is a generic method, if we're using generic sharing for the caller, but there's no generic jit info for the callee, we will not patch the call site. In this case we had Exchange<T> calling Exchange_T<T> where the callee is an icall (and evidently didn't have generic jit info). So every time we called Exchange<T>, we would go through the trampoline when trying to call Exchange_T<T> without patching the call site. Addresses part of #17334 * Bump Mono * Also drop CompareExchange_T<T> * Sprinkle some Unsafe magic. Mark the non-icall methods that we expect the JIT to treet as intrinsics with [Intrinsic] * aot-runtime doesn't need Volatile.Read<T>/Write<T> code Both of those methods are implemented as calls to the object overload of Volatile.Read/Write
lambdageek
added a commit
to lambdageek/mono
that referenced
this issue
Oct 16, 2019
Exchange<T> is supposed to be picked up as an intrinsic. But if it isn't, since T has a class constraint, call the `Exchange (ref object, ref object, ref object)` overload instead. This works around a limitation in the JIT: https://github.com/mono/mono/blob/82a07273c22b996b08fa88ee2e6632d782ac2242/mono/mini/mini-trampolines.c#L704-L713 if we're in the common call trampoline, and the caller is generic method and the callee is a generic method, if we're using generic sharing for the caller, but there's no generic jit info for the callee, we will not patch the call site. In this case we had Exchange<T> calling Exchange_T<T> where the callee is an icall (and evidently didn't have generic jit info). So every time we called Exchange<T>, we would go through the trampoline when trying to call Exchange_T<T> without patching the call site. Addresses part of mono#17334
lambdageek
added a commit
that referenced
this issue
Oct 23, 2019
…ect (#17372) * [bcl][jit] implement Interlocked.Exchange<T> in terms of object Exchange<T> is supposed to be picked up as an intrinsic. But if it isn't, since T has a class constraint, call the `Exchange (ref object, ref object, ref object)` overload instead. This works around a limitation in the JIT: https://github.com/mono/mono/blob/82a07273c22b996b08fa88ee2e6632d782ac2242/mono/mini/mini-trampolines.c#L704-L713 if we're in the common call trampoline, and the caller is generic method and the callee is a generic method, if we're using generic sharing for the caller, but there's no generic jit info for the callee, we will not patch the call site. In this case we had Exchange<T> calling Exchange_T<T> where the callee is an icall (and evidently didn't have generic jit info). So every time we called Exchange<T>, we would go through the trampoline when trying to call Exchange_T<T> without patching the call site. Addresses part of #17334 * Bump mono corlib version * Also drop CompareExchange_T<T> * Sprinkle some Unsafe magic. Mark the non-icall methods that we expect the JIT to treet as intrinsics with [Intrinsic] * aot-runtime doesn't need Volatile.Read<T>/Write<T> code Both of those methods are implemented as calls to the object overload of Volatile.Read/Write * fix netcore build * one more IntrinsicAttribute * [bcl] Remove CompareExchange_T In #17341 we change the caller of CompareExchange_T<T>(ref T, ...) to call CompareExchange (ref object, ...), but didn't delete the declaration. It's already deleted in unmanaged and in the netcore version of this file. * [bcl] Add null reference checks to Interlocked.Exchange<T> and Interlocked.CompareExchange<T> This is to mimic the old behavior in unmanaged when the intrinsic cannot be used. The issue was detected via the coreclr acceptance test: https://github.com/mono/coreclr/blob/mono/tests/src/baseservices/threading/interlocked/exchange/exchangetneg.il * and netcore * Another Mono corlib version bump
ManickaP
pushed a commit
to ManickaP/runtime
that referenced
this issue
Jan 20, 2020
…/mono#17341) * [bcl][jit] implement Interlocked.Exchange<T> in terms of object Exchange<T> is supposed to be picked up as an intrinsic. But if it isn't, since T has a class constraint, call the `Exchange (ref object, ref object, ref object)` overload instead. This works around a limitation in the JIT: https://github.com/mono/mono/blob/mono/mono@82a07273c22b996b08fa88ee2e6632d782ac2242/mono/mini/mini-trampolines.c#L704-L713 if we're in the common call trampoline, and the caller is generic method and the callee is a generic method, if we're using generic sharing for the caller, but there's no generic jit info for the callee, we will not patch the call site. In this case we had Exchange<T> calling Exchange_T<T> where the callee is an icall (and evidently didn't have generic jit info). So every time we called Exchange<T>, we would go through the trampoline when trying to call Exchange_T<T> without patching the call site. Addresses part of mono/mono#17334 * Bump Mono * Also drop CompareExchange_T<T> * Sprinkle some Unsafe magic. Mark the non-icall methods that we expect the JIT to treet as intrinsics with [Intrinsic] * aot-runtime doesn't need Volatile.Read<T>/Write<T> code Both of those methods are implemented as calls to the object overload of Volatile.Read/Write Commit migrated from mono/mono@f017835
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Steps to Reproduce
Follow the instructions from the following page to get the flame graph on linux
https://paper.dropbox.com/doc/Profile-Mono-with-TechEmpower-benchmarks--AmsEtz3lCaMVH~kMyVQxA1CBAg-ryHeobv8okzUs1Fa9TfBG
Current Behavior
Currently, mono_magic_trampoline is being called more than expected.
Expected Behavior
mono_magic_trampoline should take a small port of time of the whole program.
On which platforms did you notice this
[x] macOS
[x] Linux
[ ] Windows
Version Used:
mono/master on Oct 7, 2019
The text was updated successfully, but these errors were encountered: