compilation performance #113
Comments
|
the first KSP KAPT graph is wrong, KAPT just gets the penalty for being the first one to run. I'll try to collect more data in room for this when we run full test suite in different orders. |
|
So I did some manual tracing for test runners (just record the time) and here are the results from running our processing abstraction tests: first kapt then ksp first ksp then kapt Btw, KSP is higher here because it runs kotlin compilation twice in most cases (due to #72) But i don't know what the number would be (definitely not half) if we properly integrated it. I do expect kotlin compilation to be slower but ~15x more than javac feels like we can find some places to improve. |
|
Caching the jar dependencies and classpath scanning is an obvious low hanging fruit and could be done in a simple way without many changes if we don't need to give the user control about caching (I don't think that ClassGraph caches anything by default). Finding the resources path can also be cached, though I doubt that it will make much of a difference. Beyond that, there are many places where directories are enumerated to gather input files; we can avoid this to some degree by keeping lists of generated files where possible. In the Can you compare the runtime when using host classpath with cached classpath scanning? I'm interested to see if that makes any difference. Caching anything internal of the compiler would require significant changes because we can no longer use the
That's weird and definitely requires some further investigation. |
|
I'm working on moving that profiling setup to my fork of KCT to reduce any overhead etc caused by the AndroidX infra. It could also make sense to use something like JMH, not sure yet. I'll try to get some baseline numbers first. I think i was wrong on |
|
i started doing something very simple (and not really accurate) here: just running the same thing 100 times w/ and w/o inherit classpath (and w/ or w/o host cache) AND also removing top & bottom 10% of results, it printed: my host cache thing is super simple just to try hence might be wrong / limited But seems like it is helping. Also, these absolute numbers are still significantly lower than what i'm observing in Room. I'll also try to add more test cases w/ java files, more kotlin files etc. Last but not least, profiling this way is not really reliable so it would make sense to consider finding a proper library (damn I really miss android benchmarking library :) ) On the other hand, i'm kind of wishing that we'll spot something big (like the classpath thing, but bigger maybe) so it wouldn't matter too much to go to that depth :). I'm mostly done for today but will try to pick this up later. |
|
I was also checking what Google compile testing is doing. Here is the difference between two classpaths: in system(google compile testing way), not in ClassGraph result: in ClassGraph but not in system: I'm not sure how correct it is to use classloader of KCT (might possibly miss ones coming from the app) but it gives the same speed benefits of caching: This might also be simply because it is a 30% smaller (62 vs 41 entries) |
|
oh also all tests passed when i moved to that class loader though i'm not sure how well that path is tested. |
What do you mean by that? It should be the same one as far as I remember. Besides referencing classes of the host project, the classpath scanning is also used to find jar dependencies (most notably the tools.jar). The idea is that the user can simply add a dependency for tools.jar to the host project with Gradle and it will be found automatically, so we should still be able to do that. |
They are not equal but in my sample i'm using the classpath from the classloader of KotlinCompileTesting hence Also, I've tried adding caching to common classpaths we find (e.g. kotlin stdlib) but it does not really make a difference once the main classpath is cached (regardless, caching them in a helper |
This PR adds caching for host classpath to avoid calling ClassGraph for every compilation. In my local tests, it saves between 50 to 80ms per compilation on my laptop, which is not necessarily much but adds up when you run hundreds of them. I've also cached common jars we find from there. It does not necessarily help with performance but rather as a cleanup to keep all of them in 1 place. Test: existing tests Issue: tschuchortdev#113
This PR adds caching for host classpath to avoid calling ClassGraph for every compilation. In my local tests, it saves between 50 to 80ms per compilation on my laptop, which is not necessarily much but adds up when you run hundreds of them. I've also cached common jars we find from there. It does not necessarily help with performance but rather as a cleanup to keep all of them in 1 place. Test: existing tests Issue: tschuchortdev#113
|
Some more interesting information from Room tests. I did a quick hack in room to Below are the results. These are not run under same circumstances so don't get attached to numbers too much but more or less, using a limited classpath shaves around 1-2 min over an 8 min test. (and once caching is in for classpath, that would mean another 50-60ms per run and we have ~ 400 compilations in this test) a) b) c) |
This PR adds caching for host classpath to avoid calling ClassGraph for every compilation. In my local tests, it saves between 50 to 80ms per compilation on my laptop, which is not necessarily much but adds up when you run hundreds of them. I've also cached common jars we find from there. It does not necessarily help with performance but rather as a cleanup to keep all of them in 1 place. Test: existing tests Issue: tschuchortdev#113
This PR adds caching for host classpath to avoid calling ClassGraph for every compilation. In my local tests, it saves between 50 to 80ms per compilation on my laptop, which is not necessarily much but adds up when you run hundreds of them. I've also cached common jars we find from there. It does not necessarily help with performance but rather as a cleanup to keep all of them in 1 place. Test: existing tests Issue: #113
|
so adding some more code in room to prevent KCT from resolving all classpath also has some minor improvements: |
|
btw here is a profile in KCT after the caching changes. Pretty much all time is spent in kotlin compilation so i don't think we will be able to do much here unless we figure out how to run kotlin compiler more efficiently. Maybe there are ways to reuse project model etc. Btw, KSP tests, which run similar, run a lot faster. Though they possibly just analyze without really compiling so it might be apples to oranges. |
This CL implements two optimizations for KCT. a) Right now, it resolves the host classpath for each run, taking 50-60 ms for each compilation (and we run ~3 of them per test). I've added caching for common jars that it resolves from host classpath so that it does not need to resolve it for each test. b) When we inherit classpath (which we always do), the size of the classpath seems to have a signficant impact on kotlin compilation time. Instead, this CL picks a filtered view from the whole host classpath. (see linked issue for measurements). After these changes, TableEntityProcessorTest (which runs ~400 compilations) takes ~4:30 min instead of ~8 min on my machine. tschuchortdev/kotlin-compile-testing#113 Bug: 160322705 Test: existing test. Change-Id: I8a738c76879d44a31bb22b050466a2dc49ef45e1
|
some more info: Looks like this is what gradle does for build.kts files: It does require some more lifecycle management but seems to help in Room when I set that env property via gradle task. |
|
hmm a simple test to enable it didn't help. |
|
Are you sure that this env variable has any effect when we are using the embedded compiler like this? Maybe we should ask someone on the compiler team if they have any ideas how to get some cheap speed-ups. |
|
actually i might've found something. Then I started adding more logs inside KCT and discovered that java compilation actually takes longer than kotlin compilation. It is no surprise this affects room more as most of our tests are with java sources. Appearantly, we do create a process for java compilation which might be why it is slower. Any particular reason for that? This is what i'm thinking of using: |
|
btw here is that logging code: |
|
hmm actually we do use a process only if java home is set but since it has a default, it would (almost?) always be set unless we fail to find jdk home from current classpath. |
|
here are some measurements w/ an additional flag to use the in process java compiler: So when java files are involved, this has a huge impact. Here is a recommendation:
Alternatively, we could just introduce another parameter for in process compilation. @tschuchortdev any preferences? |
|
ok it fails with So it seems very intentional though idk if anyone relies on this behavior. Compiling java w/o boothclasspath seems weird, was there a particular use case for that? That test also intentionally sets jdkHome to null so maybe it makes sense and the right solution is to literally check |
…eveloper This PR updates KCT to use in process compilation unless the developer explicitly provides a jdkHome that does not match the java home of the current process. In most cases, it won't be provided and because we have a default, current implementation makes KCT spin off another process to compile java sources, causing a significant compilation time penalty. tschuchortdev#113 (comment) With this change, KCT won't create another process unless jdkHome is set to a value that does not match the jdk home inherited from the current process. Issue: tschuchortdev#113 Test: As this is a bit difficult to test, I've added test that will check the logs ¯\_(ツ)_/¯
…eveloper This PR updates KCT to use in process compilation unless the developer explicitly provides a jdkHome that does not match the java home of the current process. In most cases, it won't be provided and because we have a default, current implementation makes KCT spin off another process to compile java sources, causing a significant compilation time penalty. tschuchortdev#113 (comment) With this change, KCT won't create another process unless jdkHome is set to a value that does not match the jdk home inherited from the current process. Issue: tschuchortdev#113 Test: As this is a bit difficult to test, I've added test that will check the logs ¯\_(ツ)_/¯
…eveloper This PR updates KCT to use in process compilation unless the developer explicitly provides a jdkHome that does not match the java home of the current process. In most cases, it won't be provided and because we have a default, current implementation makes KCT spin off another process to compile java sources, causing a significant compilation time penalty. tschuchortdev#113 (comment) With this change, KCT won't create another process unless jdkHome is set to a value that does not match the jdk home inherited from the current process. Issue: tschuchortdev#113 Test: As this is a bit difficult to test, I've added test that will check the logs ¯\_(ツ)_/¯
…eveloper This PR updates KCT to use in process compilation unless the developer explicitly provides a jdkHome that does not match the java home of the current process. In most cases, it won't be provided and because we have a default, current implementation makes KCT spin off another process to compile java sources, causing a significant compilation time penalty. tschuchortdev#113 (comment) With this change, KCT won't create another process unless jdkHome is set to a value that does not match the jdk home inherited from the current process. Issue: tschuchortdev#113 Test: As this is a bit difficult to test, I've added test that will check the logs ¯\_(ツ)_/¯
I don't remember exactly why I did it that way (probably should've documented it better) but I think it was mostly to be consistent with the behaviour of kotlinc when It says in the doc comment of My guess is that nobody uses this feature but since there was a
Starting javac in another process when |
|
BTW does Room have any tests that have only Java sources and no reliance on the Kotlin compiler? If I'm not mistaken, the |
…eveloper This PR updates KCT to use in process compilation unless the developer explicitly provides a jdkHome that does not match the java home of the current process. In most cases, it won't be provided and because we have a default, current implementation makes KCT spin off another process to compile java sources, causing a significant compilation time penalty. #113 (comment) With this change, KCT won't create another process unless jdkHome is set to a value that does not match the jdk home inherited from the current process. Issue: #113 Test: As this is a bit difficult to test, I've added test that will check the logs ¯\_(ツ)_/¯
We've started running more tests of Room with KCT (each test is run w/ java compile testing, KCT/KAPT, KCT/ KSP) and saw a significant slow down in our tests.
Creating this issue to track that work to see if there are places where we can improve in the library.
I started profiling them and seems like most time is spent in KAPT. In fact, KSP path is almost equal to Java compile testing path whereas KAPT runs are ~5x slower.
Sample run for the same test:
And for whatever reasons, commonJvmArgs is taking a long time in KAPT's stubs and apt task but not in KSP. (specifically, parseCommandlineArguments)
When
commonArgumentsis invoked for KAPT:When
commonArgumentsis invoked for KSP:I've not yet debugged what is going on there.
Another penalty is the classgraph / jar searches.
Right now, each KotlinCompilation instances re-acquires the class graph and find jars but they could be cached since they won't change per process.
As a result, it dominates the time spent in tests. I'm taking a small test sample so this might also be already cached in ClassGraph side so it might be an amortized cost but in my small test sample, it spend 30% much time as running actual tests (where compilation happens)
Below are two profile outputs (html and interactive svg). FYI they are from different runs of the same task.
There is a lot of overhead from gradle there since i'm not running many tests in the batch so please don't pay attention on total time ratios.
profile.html.tar.gz
profile.svg.tar.gz
I'll try to share more and hopefully come up with some solutions.
The text was updated successfully, but these errors were encountered: