[llvm-commits] [PATCH] Performance degradation when repeatedly exchanging JITted functions

[Clemens Hammacher]

Hello,

some of you may already have read this on the LLVMdev list, but let me 
shortly recapitulate the problem:
When repeatedly calling recompileAndRelinkFunction on the same function, 
the global mapping is always updates with the new version, and at the 
memory location of the old version a jmp is added. This builds long 
chains of jmps, which lead to significant performance degradation, even 
if only exchanging several times (47% performance after exchanging 5 times).
Another problem is that the memory of the old version can never be freed 
since it participates in the jmp-chain. The current patch does not 
tackle this memory issue though.

The solution that this patch implements is the following:

The original stub (which is always being hold by the JITResolver) is 
updated to point to the new version in any case.
Additionally you can set a flag in the ExecutionEngine to always use the 
stub when calling a function. If this flag is set, a 
recompileAndRelinkFunction does *not* patch the old function pointer to 
jump to the new function, since all calls use the stub anyway.

Since several places in the JIT itself rely on the global mapping being 
updated to the start of the newly jitted function, I didn't change that. 
Instead, after jitting a function, the mapping is changed back to the 
stub, if the KeepStubs flag is set.
The only drawback of this is that *directly* recursive calls still 
bypass the stub and jump back directly to the function pointer. To 
estimate how important that issue is you have to distinguish two cases: 
Either one thread wants to exchange a function that is concurrently 
being executed by another thread. Then you run into a lot of other 
problem too, since the whole jitting and memory patching is 
unsynchronized to running code. The other case would be that a thread 
wants to exchange a function that he is currently executing, so that 
further recursive calls use the new version. This is unsafe also in the 
current implementation, since you would overwrite the memory of the 
function that is being executed. So at least it is no regression from 
the current implementation. Nevertheless, it may be allowed later, after 
changing larger parts of the code.

So I think this should be fine for now.

I attached a patch implementing this, and a test case for the new flag. 
Both apply to trunk.

While implementing this, I also stumbled across another bug, so whoever 
commits the attached patches may also just have a look at the fix and 
the testcase I attached to that bug report, since it is slightly related.
http://llvm.org/bugs/show_bug.cgi?id=12197

If you comment on this patch, please also include me personally, since 
I'm not subscribed to llvm-commits.

Thanks,
Clemens
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: implement_KeepStubs.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120307/8d286003/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: testcase_KeepStubs.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120307/8d286003/attachment-0001.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 6392 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120307/8d286003/attachment.bin>