-
Notifications
You must be signed in to change notification settings - Fork 18
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
exploding compile times post development branch merge #158
Comments
There appears to be a number of moderate perf regressions compounding to larger inefficiencies. I pushed a couple commits today:
Let me know how much your perf improves, on my side I got the following numbers:
I'll update this post if/as I find other low-hanging fruit to optimize. |
I did a quick and dirty benchmark as well on Ignis:
Note: The benchmarks are single-threaded, release build, run on a clean cache and contain a set of 10 shaders (large artic files). I used the diamond_scene.json for this benchmark. * This required Ignis master branch, instead of the development branch. But the code-complexity did not change between these. |
Awesome, thanks! It's still about 5× slower (25 s vs 5 s) on my minimal test build here than pre-merge, but that's already a lot better than 27× slower 😄👍 |
The changes incorporated with 62311a4 result in a massive increase in compile time. Compared to 47f4b88, we see compilation time for a random anyq benchmark rise from less than 1.5 seconds to over 7 seconds. The entire build slows down by over 27×, which makes this effectively a blocking issue, as what used to take < 3 min would now take well over an hour.
Based on some initial
perf
data (see below), it seems thatartic
post development branch merge spends almost half of the entire build time just rehashing hash tables during rewriting.THORIN_PROFILE
does not yield any warnings regarding collisions, so this appears to be a consequence of the changes rather than just some issue with the hash tables.repro.art, compile with
--emit-llvm
to reproduce problemnew (t = 7.2 s)
old (t = 1.4 s)
The text was updated successfully, but these errors were encountered: