-
-
Notifications
You must be signed in to change notification settings - Fork 64
Performance degradation with 1.4.0 #166
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
Comments
I've also seen a couple of performance issues today - sadly one day after release :-/ @tmohme Can you share one or two examples that make the problem obvious? |
I found a strong correlation of slowness with usage of custom Arbitraries for heavy domain objects. |
@tmohme Do you use combinators in you domain arbitraries? That’s what I found at first glance. |
Yes, extensively (at least for these heavy domain objects). |
@tmohme That gives me some guidance where to look. |
@jlink Will you be OK with a Kotlin example? |
@jlink sure |
A small gradle project containing a property-test using an arbitrary-provider for a heavy domain object: |
@tmohme Thanks! |
@tmohme Can you try "1.4.1-SNAPSHOT"? It should get rid of the performance degredation; as a matter of fact it should speed up generation compared to 1.3.10 quite a bit. Overall test time of jqwik got down by almost 25%. Here's the downside: The fix completely breaks the behaviour of It feels like a Catch-22: Either break compatibility despite all the promises I signed with my blood, or leave in the really bad generation performance for composite and flat-map arbitraries, or invest days if not weeks in a deprecated and soon-to-remove functionality. |
@osi That’s actually a good idea. Let’s see what’s possible with limited effort... |
@osi Turns out, even a "well enough" implementation requires more effort than I'm willing to invest. I'll go ahead with a 1.5.0 soonish. |
@tmohme I ran your example with 1.3.10 and 10000 tries: 6.9 sec. With 1.5.0-SNAPSHOT: 6.8 sec. Seems 1.5.0-SNAPSHOT should have fixed your problem. So I'll probably go for a 1.5.0 release tomorrow. |
@jlink I think there is no need to rash this decision to drop backward compatibility so fast:
|
Just ran some timing tests.
I don't have any numbers for executed tests for 1.4.0 because I started the series with 1.4.0 and noticed only after a few runs that the number of executed test vary. |
@tmohme Looks like an interesting riddle :-/ There’s obviously more going on... |
I'll try to look a bit broader (with gradle) on our tests find out which tests are affected most. |
@tmohme Take your time. I decided not to rush 1.5.0. |
I repeated the timing series from above with gradle and a slightly different source state:
edit: new column "1.5.0-SNAPSHOT (81fb971)"
Next I'll have have a closer look into 1.5.0-SNAPSHOT. |
@tmohme My guess it that most of the time is spent creating the generator with edge cases. |
Investigating all test durations with gradle I found that the slowest test are (as far as I can see) not structurally different to what I provided as a demo I provided you, although they might be even heavier in terms of "number of combined fields". Internally caching the arbitraries definitely helps a lot. The test-group from which I derived the demo is even fast with with 1.5.0-SNAPSHOT (~6s) than with 1.3.10 (~8s). |
A cpu flame graph of the first 60s of execution of one of my slow tests (tries=10000) with 1.5.0-SNAPSHOT: I guess the column with the hover-note is where the actual test execution happens. |
Apart from the different relations, it is conspicuous that the call stack seems to be quite a bit deeper with 1.5.0-SNAPSHOT, which could (at least partly) be one reason for the shifted relation. |
@tmohme Thanks for the detailed analysis. One last question: Is execution time still growing in an overlinear way? |
No, it seems to be linear with 1.5.0-SNAPSHOT.
edit:
Not all tests are exactly comparable (in terms of parameters and executed logic) and the test execution order varies between runs, but nevertheless it is safe to say that in this setup 1.5.0-SNAPSHOT is slower by a factor of ~3. |
@tmohme You might want to try the latest 1.5.0-SNAPSHOT (make sure you refresh dependencies). Your demo runs twice as fast as with 1.3.10 here. |
I can confirm 1.5.0-SNAPSHOT (c00d945) is way faster than the previously tested versions. The execution time now seems to grow sub-linear, converging to linear. Probably because it starts slow, followed by significantly faster executions due to cache hits. The cpu profiling confirms that now the time spend for actually executing the test functions dominates the preparation part: edit: |
@tmohme I take it you'd be satisfied with the performance as it is? |
That's right 👍 |
Using memoization, the performance looks fine in 13e8ed7 |
Using 1.4.0 (as opposed to 1.3.10) we experience a severe performance degradation compared to 1.3.10 for some tests.
The time required to execute (at least some tests) seems to increase super-linear with the number of tries.
According to a first quick glance
edgeCases
are heavily involved.I'll try to investigate this further and/or come up with a reproducer in the following days.
One of the affected test classes (containing 17 tests) shows the following behavior:
The text was updated successfully, but these errors were encountered: