https://kotlinlang.org logo
Join Slack
Powered by
I've been doing some digging into some of the <slo...
# kotest-contributors
a
I've been doing some digging into some of the slow tests and I found something interesting that I hope someone can double check for me. ForAllExhaustivesIterationTest#forAll with 21 exhaustives should run for each cross product is slow, taking over 4 mins on CI I ran the IntelliJ profiler, and I found that the main cause of this slow speeds appeared to be because the seed file was being deleted with 
deleteRecursively()
, even though it's a file, not a directory. https://github.com/kotest/kotest/blob/a95078d987f5416f815f96d292a80407c9242277/kotest-property/src/jvmMain/kotlin/io/kotest/property/seed/seedio.kt#L55 So, I replaced it with 
f.deleteIfExists()
, but it was still causing a lot of slow down. I then checked where 
clearSeed()
is being called from, and the only non-test invocation is in 
test()
https://github.com/kotest/kotest/blob/42af0ff23307398e2723618449feda0f5646352e/kotest-property/src/commonMain/kotlin/io/kotest/property/internal/test.kt#L52 This looks suspicious to me, because it looks like 
clearSeed()
is always being called, while I would expect that it should only be called if the test fails. Is my understanding correct? In any case, even if the seed should be cleared after a successful test, it is probably not good that Kotest attempts to delete the seed file so often, even if it doesn't exist. Is there a better way of doing this?
here's the CPU flamegraph with 
deleteRecursively()
and with the update to 
deleteIfExists()
, which is still slower than I'd like
Found the answer to my questions. > This looks suspicious to me, because it looks like clearSeed() is always being called, while I would expect that it should only be called if the test fails. Is my understanding correct? My understanding wasn't correct. The seed should be deleted after a successful test. > Is there a better way of doing this? Yes, instead of eagerly deleting the file, instead mark files for deletion in-memory (in a mutable set) and delete them when the test has completed. WIP solution here https://github.com/kotest/kotest/pull/4183
s
I'm confused. Why would writing a few seed files cause a dramatic slowdown ?
a
I am not completely certain, but it's not writing files that is a problem, it's checking if it exists before deleting it
and it's not just a few seed files, it's checking if a single seed file for the current test exists over 4 million times (in the 'forAll with 22 exhaustives...' test)
new build scan for PR 4183 shows 
forAll with 22 exhaustives should run for each cross product
is 2x faster. 4m33s vs 2m20s
s
Is it perhaps checking the seed file on every iteration instead of just once. There isn't 4 million tests but might be 4 million iterations.
a
yes that's right
s
The better fix would be for that then maybe
a
I am very interested in better fixes 🙏
s
Clear failed seed can move out of there to the place that invokes test
See if that plus your change improve on the 2m
a
it would be nice to make it faster! But when I profile the tests I don't see any obvious places to improve
s
Clear failed seed defiantly only needs to run once before the start of the iterations right.
Won't make it slower to do that
But maybe you made that method so quick it won't improve it further either
a
I think that the slowest part is checking if the result of 
property(...)
is a boolean, since 
shouldBe
uses generics and has branches
Clear failed seed defiantly only needs to run once before the start of the iterations right.
So, PBT needs to load the seed (if it exists) before the iterations start. Could it be a problem if PBT loads the seed, then deletes the file, and then the entire process crashes? The seed would be deleted, even though it failed.
s
I think it's fine to nuke the seed first
This is test code not production
a
true
although, it should be easy to just move the 'delete seed file' code to be done after all PBT iterations are done
all that's needed is the TestPath...
that would make my PR simpler
s
Yeah that works
a
I think that the slowest part is checking if the result of 
property(...)
is a boolean, since 
shouldBe
uses generics and has branches
I've looked into this more. I don't think 
shouldBe
is slow. I guess it's just that the testFn is a suspend function, so launching it is slow? Also, 
BeforePropertyContextElement
and 
AfterPropertyContextElement
are a bit slow. But I think they're being deprecated?
s
Once we split out the property test stuff I'm going to revisit property test hooks for 6.0
I can't split the projects though while there are 30 PRs in flight as it would break too many prs
my linux machine really doesn't like the size of our project. It's just completely killed my machine again running check
a
whoa, crazy
that sounds like something more significant than the size of the project... I wonder
s
its all the native compilation
it doesn't like doing the linux native builds
a
huh, interesting...
usually it's Mac machines are slower, since they build all targets
s
yeah but people tend to have powerful mac's using M3 or whatever
not my pentium 100 linux machien
256Mb ram
1gb hard drive
a
whoa yeah, okay, I can see how those specs would be a factor
s
I'm exaggerating how old it is
but it's slow
a
how is performance if you run 
./gradlew jvmTest
?
s
still slow
I tend to run from intellij tho
a
could you share a Gradle build scan? I can take a look and see if anything jumps out as a quick fix
s
sure, remind me how to trigger a build scan again
--scan ?
a
yes
something I'm waiting for is Gradle to finish implementing is automatically setting the JDK used to run the Gradle daemon. At the moment Gradle just picks JAVA_HOME, and if that's different to what's used on CI to populate the remote build cache, then there will be a lot of cache-misses. If the JDK is stable, then that would mean more cache-hits.
that's not a complete fix, but it'd help
s
didn't they adjust that in 8.9?
a
yeah there's an option, but it's incubating, and it doesn't auto-download missing toolchains (although it does auto-discover them)
s
ah ok I was thinking of this I think
a
in my 
~/.gradle/gradle.properties
I set java home, which I think helps
Copy code
# Try to help with Build Cache re-use by always using the same JDK
org.gradle.java.home=/Library/Java/JavaVirtualMachines/temurin-17.jdk/Contents/Home/
s
how would my local JAVA_HOME be affected by the remote cache ?
a
it's the other way around - different JDKs mean different cache entries
s
you mean if different JDKs are used for building locally, then the local cache isn't going to be as effective?
a
If CI uses Java 17 to run Gradle, then the buildscript classpath is considered to be completely different to if I locally use Java 21. The buildscript classpath is part of the cache-key computation for basically everything.
yeah exactly
it doesn't break anything, but it does hurt
s
I'm confused though like why would I care what the CI is doing? Like how does it affect my local build speed ?
a
CI populates the remote build cache, which your machine can read
s
oh that's interesting
makes sense now
a
in theory, if the master branch is green, and you do a fresh checkout and run 
gradle check
, then all tasks should be instantly up-to-date!
👍🏻 1
s
my machine might not be taking advantage the latest remote cache entries
a
exactly
s
so once that new gradle feature is ready, you can be explicit about the version of the JDK used for both the gradle deamon, and the java tasks it forks
a
yup!
s
clever
a
yeah, Gradle can be really great sometimes
s
its super complicated though
💯 1
I feel that the gradle DSL is the worst DSL ever written. But that's partly because it all started out in groovy and they took "advantage" of some groovy's quirks. By moving to Kotlin its becoming gradually more clear.
a
definitely
s
it failed on a test tho
lol my machine just tanked again, had to power off power on. I really want to split this project out tonight.
a
hmm I don't see anything obviously wrong in the build scan, apart from you're using Java 21
using Java 17 (the same as the GitHub runners) would help, but not significantly
s
Just my rubbish machine then
And all the expensive native targets
a
you could try limiting the Gradle workers - add this to 
~/.gradle/gradle.properties
Copy code
org.gradle.workers.max=2
s
I have 32 cores
But I will try that
a
by default max-workers will be 32 then, but yeah, you can play around with the number
it's not a great workaround though
s
Better than a forced reboot though lol
a
haha yes
I'm surprised that the native targets are so heavy though...
do you get similar problems on other projects?
🚫 1
the build scan only sees 16 cores 🤔 https://scans.gradle.com/s/enrjchrn2ivlu#infrastructure
s
Maybe I only have 16 I'll check again
Its an i9
a
yeah, so 32 cores...
I wonder if something is misconfigured in the hardware? But that doesn't really make sense.
I'm just thinking about how sometimes the RAM clock speed is configurable and sometimes people forget to set it correctly
s
I can build other giant projects like intellij
a
the GitHub runners have 4 cores so it really shouldn't be a problem
hmmmmm interesting
do the crashes only happen when running Gradle through IntelliJ? Could that be related?
because yeah, IntelliJ is huge
s
Happens in regular gradle too
Its intermittent
a
hmm okay, so I think it's either a problem with Gradle or with Kotlin Gradle Plugin 🤔
possibly the Kotlin compiler daemon
if you have time, could you make a report? https://kotl.in/issue
s
you think it's an issue with the kotlin compiler vs my machine just being underpowered for the size of the project ?
a
yes
the GitHub Action runner machines have less resources, and while they're slow, they don't crash
maybe it's a hardware issue. If it's an older laptop, maybe something is loose or it overheats?
s
It's a desktop, and the gfx card is truely shit. I think perhaps when I'm trying to do too many things inside xfce it just gives up.
a
but I think it's more likely to be something with Gradle or Kotlin, since you can build IntelliJ
Maybe enabling Kotlin build reports might reveal something https://kotlinlang.org/docs/gradle-compilation-and-caches.html#build-reports
s
ok let me give that a go and do a full build scan
a
xfce.... I wonder if it could be a problem with the JS tests using headless Chrome? Since that launches a new application.
s
perhaps
I'm running an old ubunutu too
22.04
a
ah okay, that should be fine? I don't really know though
s
should be since it's LTS
a
yeah, definitely
s
I still wanna split the project up though, it's too complicated for people to dive in given the 40 modules IMO
a
Gradle has some sort of a VFS watch, but that should be fine. But I guess you could try disabling. https://docs.gradle.org/current/userguide/file_system_watching.html#supported_operating_systems
s
I'm trying to fix something atm, so will run these suggestions later this evening. Don't want my machine to crash mid zone 🙂
a
for sure!
I work on the Kotlin Gradle Plugin team, so I'm doing my part in trying to find problems 😄
s
oh you do
that's pretty cool
anyone at jetbrains use kotest?
a
umm kind of, I know Arrow uses it
I'm merging Dokkatoo into Dokka, and that uses Kotest
s
not heard of dokkatoo
I guess its a play on dokka2 ?
a
exactly
basically the official Dokka Gradle Plugin is not very compatible with Gradle. It's quite outdated. So I wrote Dokkatoo, just as a better wrapper around the actual Dokka Engine
s
ok makes sense
I'm excited to get pushing on kotest 6
I have a much better intellij plugin experience almost ready to go
a
that's fantastic
going back to the original topic, 
forAll with 22 exhaustives should run for each cross product
now runs 4 times faster than before! 4m33s vs 1m00s https://scans.gradle.com/s/42qggo5p6kaxm/tests/slowest-tests#forall-with-22-exhaustives-should-run-for-each-cross-product-2
s
Sick
4 Views
Open in Slack
PreviousNext
Powered by