-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
O(n^2) scaling in llvm symbol lookup #15619
Comments
Nice detective work as always, @yuyichao. |
it's probably worthwhile to memoize the result of |
i've created a version of yuyichao's script that also emits a plot and shows my fix will be O(1): https://gist.github.com/vtjnash/89cff0605d29fe968e56 (with total compile time const on this benchmark roughly 2x the old jit, but at least with constant scaling now) |
CompileLayer.findSymbol is O(n) in the number of modules that have been emitted but we can pre-compute the result of findSymbolIn when notified that an object has been emitted and store it in one hash table for the JIT fix #15619
CompileLayer.findSymbol is O(n) in the number of modules that have been emitted but we can pre-compute the result of findSymbolIn when notified that an object has been emitted and store it in one hash table for the JIT fix #15619
Reported upstream https://llvm.org/bugs/show_bug.cgi?id=27188 |
While fiddling #14846 with gdb folks I noticed that there's a
O(n^2)
scaling in our own JIT too.The test is done on patched llvm 3.7.1 with this script which is basically emitting a lot of small functions with different names by,
Ploting the time vs
n
on a log-log scale it's very clear that theO(n^2)
scaling takes over forn > 10k
.The profile (done with
perf
since I was profilinggdb
with the same command) withn = 50000
is,and it seems that most of the time is spent in the symbol lookup (some of the other functions are also quite significant but the symbol lookup seems to be the worst).
@vtjnash and @carnaval suggested that llvm has a
O(n)
object file look up and we might need to replace it with our own hash table injitlayer.cpp
.Also c.c. @Keno
The text was updated successfully, but these errors were encountered: