[Mono-devel-list] method recompilation - looking for feedback

[Willibald Krenn]

> I'm not sure what you mean, but if CEE_LDFTN or CEE_LDVIRTFTN
> have been seen, it's not safe to remove a method's code, since the
> resulting pointer could have been stored anywhere.

This was more or less the look at the problem from the spec. standpoint.

I meant that if all the CEE_LDFTN pointers are used to create delegates, 
it's safe to move that method. (Because we know where the pointer went 
to, how it will be used and we can potentially change the delegate 
invoke sequence..)
I did not know that mono always creates a trampoline for these opcodes.

So if I'm not mistaken a delegate invoke call might look the first time

Invoke->Tramp(unbox)->Tramp(jump)->Target

and the second time

Invoke->Tramp(unbox)->Target

I wonder if the jump trampoline is ever freed, because if the LDFTN 
pointer was taken for e.g. calling from native code, it must not be 
freed at all.
But I guess freeing the jump trampoline is illegal because in case of a 
'missing' unbox trampoline, the delegate target pointer does not get 
updated by the tramp. code and is directly used by the dynamically 
created invoke method to jump to the target function... (I may be wrong 
of course, but it seems that a simple delegate call always goes over (at 
least) one tramp.)

>>Q: How to track delegates?
>>A: This is a little bit more tricky. Basically it could be done in sync 
>>with GC or by including some list in MonoJitInfo.. While the latter 
>>still needs support for delegate-cloning (etc), the former needs 
>>modifications to the GC. Any ideas which one would be easier/better?
> 
> 
> For methods that can be dynamically recompiled or whose address is taken
> with ldftn-like instructions, we need to keep the trampoline around
> and patch it to jump to the new code.


Given the invoke example from above, a moved method would then be 
invoked as (worst case)

Invoke -> Tramp(unbox) -> Tramp(jump) -> Tramp(moved) -> Target

which would be converted to

Invoke -> Tramp(unbox) -> crash

if the Tramp(moved) was freed because a 'RefCounter' (# of callers 
successfully patched) decremented to 0; I guess a Tramp(moved) without a 
reference counter doesn't have that much sense, so I'll look into that 
as well.


Of course trampoline methods must not be moved at all..


> 
>>Q: How could raw IP -> MonoJitInfo ptr translation be more efficiently 
>>made from within signal handler?
> 
> 
> This has a surprising cost in exception handling, too (the profiler signal
> handler should likely just store the ip and not do much processing).

Yes, this is how it's currently implemented. But at some point one has 
to translate the raw IP, so ..

>>A: I thought of adding a pointer to the MonoJitInfo structure -ptr_size 
>>bytes before the first opcode of the method. Within a signal handler it 
>>should be quite easy to obtain the starting address of the current 
>>method as bp-1 points to after the method call opcode and e.g. on amd64 
>>one opcode before r11 gets loaded with the call target...
> 
> 
> This is not a solution: the amd64 call sequence needs to be changed to
> not do that, since it's too slow (and you wouldn't handle virtual calls
> that way etc).

Yes, I forgot about these :(

> For a number of reasons, the jit info lookup should also become
> lock-free, so ideas and patches to make this function faster are welcome.

I still think a direct pointer to the jit info one ptr_size before the 
method start would be a good idea. If the starting address is aligned to 
8 bytes, the search should not be that time-consuming.
Alternatively the arch dependant part could do some quick disasm (check 
different call opcodes to gain the pointer location). Either way, if the 
method was aot-compiled, a 'negative' jit-info pointer could indicate an 
indirect load.. And if the jit info is guaranteed to be 8 byte aligned, 
the 3 'remaining' bits of the pointer could be used for a quick validity 
check. (e.g. (jit_info_ptr >> 3) % 8 )


> I don't see the need to expose MonoJitInfo to C# code, we're also not
> going to make that structure larger: it needs to remain light-weight
> for the default case (it will actually be shrinked).

I really see no way other than storing some information in the jitinfo 
struct for doing method-weight calculation efficiently. If not saved 
there, I need another datastructure plus pointer which - in sum - would 
be more bloat than expanding MonoJitInfo. But this additional 
information is only necessary when mono is built in some 
method-recompilation profile, so it won't harm the 'default case'..
BTW: The additional info is 64 bits in size (size of a pointer here): 
Mostly because I'm using 16 bit variables for the weight and generation 
count. As always it's a memory/speed tradeoff: I could, possibly, go 
down to 32 bits, but that would mean
  - weight 8 bit (this would hamper functionality as well)
  - generation 8 bit
  - no fast way of updating the hitlist: Would lead to linear searches 
and g_array_remove calls instead of the fast ones.


Willi