Skip to content
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

Improve the performance of methods SoftAssertions#assertThat #2563

Open
JohnnyBFG opened this issue Apr 12, 2022 · 11 comments
Open

Improve the performance of methods SoftAssertions#assertThat #2563

JohnnyBFG opened this issue Apr 12, 2022 · 11 comments
Labels
status: pending investigation An issue that needs additional investigation theme: soft assertions An issue related to the soft assertions
Milestone

Comments

@JohnnyBFG
Copy link

JohnnyBFG commented Apr 12, 2022

Summary

We recently upgraded from AssertJ 3.8.0 to 3.22.0 and we noticed significant slow down in tests using SoftAssertions#assertThat.
I look into it and found that AssertJ switched from cglib to ByteBuddy in 3.9.1.
I did some simple performance corporation between version and found that proxy generation is ~4x slower compare to AssertJ 3.8.0. with cglib. Speed difference is quite noticeable especially on first running test. TypeCache in SoftProxies is kinda helping, but only tests running after type was already cached.
I also compare times from AssertJ 3.22.0 to Mockito 3.2.4 (which is quite old, but it's too using ByteBuddy to generate sub-classes) and found that execution times for generating classes are almost identical.

This slowdown make SoftAssertions#assertThat unsuitable for some of our tests, as they must run under 2s + it makes harder to predict actual execution time of whole test. We found workaround in SoftAssertions#check(() -> assertThat) but this syntax can be harder to read in longer tests so we would prefer to use SoftAssertions#assertThat as it's more natural.

I want to ask you if you could please look into this problem and found if there is any space to improvement?

I understand that there isn't probably any space on ByteBuddy side for improvements, but maybe there could be done something on ours?
Maybe change some strategy or use ByteBuddy to generate class from interface (not actual class) and delegate calls to real object (but that would require create interfaces for each Assert class and I'm not sure if it would be possible due generic + if such change wouldn't result if worse runtime performance).
This problem can be also environment problem and sub-class generation is much faster on newer versions of JDK (but I have doubts about that, I experienced similar problems on up-to-date versions of JDK 11 too).

Testing environment

java version "11.0.12" 2021-07-20 LTS
Java(TM) SE Runtime Environment 18.9 (build 11.0.12+8-LTS-237)
Java HotSpot(TM) 64-Bit Server VM 18.9 (build 11.0.12+8-LTS-237, mixed mode)

Tests were performed in Intellij Idea 2020.2.4

AssertJ performance comparison

Used tests:

//used for AssertJ
public class MainTest {
    private final Exception exception = new Exception("lol");
    private final Map<Object, Object> map = Map.of();
    private final Object object = new Object();
    private final List<String> list = List.of();

    @Test
    public void text() { SoftAssertions.assertSoftly(softly -> softly.assertThat("text")); }
    @Test
    public void object() { SoftAssertions.assertSoftly(softly -> softly.assertThat(object)); }
    @Test
    public void list() { SoftAssertions.assertSoftly(softly -> softly.assertThat(list)); }
    @Test
    public void optional() { SoftAssertions.assertSoftly(softly -> softly.assertThat(Optional.empty())); }
    @Test
    public void exception() { SoftAssertions.assertSoftly(softly -> softly.assertThat(exception)); }
    @Test
    public void booleanAssert() { SoftAssertions.assertSoftly(softly -> softly.assertThat(Boolean.FALSE)); }
    @Test
    public void map() { SoftAssertions.assertSoftly(softly -> softly.assertThat(map)); }
}

//Mockito 3.2.4
public class MockitoTest {
    private final Exception exception = new Exception("lol");
    private final Map<Object, Object> map = Map.of();
    private final Object object = new Object();
    private final List<String> list = List.of();

    private static <T> T mock(Class<T> classToMock) { return Mockito.mock(classToMock, InvocationOnMock::callRealMethod); }
    @Test
    public void text() { mock(StringAssert.class); }
    @Test
    public void object() { mock(ObjectAssert.class); }
    @Test
    public void list() { mock(ListAssert.class); }
    @Test
    public void optional() { mock(OptionalAssert.class); }
    @Test
    public void exception() { mock(ThrowableAssert.class); }
    @Test
    public void booleanAssert() { mock(BooleanAssert.class); }
    @Test
    public void map() { mock(MapAssert.class); }
}

Results:
AssertJ 3.8.0
image

AssertJ 3.22.0
image

Mockito 3.2.4
image

@filiphr
Copy link
Contributor

filiphr commented Apr 12, 2022

Perhaps an option for AssertJ would be to generate the proxies for the known classes during build time. Not sure what kind of an impact this can have on the JAR size of AssertJ.

Apart from that not sure if @raphw might have some better ideas for how to improve the performance. Perhaps there are some things that can be done in AssertJ, that we don't know about.

@raphw
Copy link
Contributor

raphw commented Apr 12, 2022

Are you running single tests? In this case, it takes longer indeed. The "problem" is that Byte Buddy resolves generics and bridges to correctly proxy types. Even if such type information is not present, it is resolved lazily what requires I/O and adds to the total balance.

If you run a largish suite, this should not normally be a problem. Have you tried running with -Dnet.bytebuddy.raw=true? This would disable generic type resolution but can cause weird edge cases.

@imparente
Copy link

@raphw
a) Our builds are creating a new JVM every now and then for historic reasons that are beyond this conversation. It just needs to be done until we figure out a better solution for a completely different problem.
b) We want to encourage people to write proper and well isolated unit tests that should run in milliseconds. And one of the ways how our company chose to enforce this practice was that they have introduced a TimeOut mechanism that guards that a unit test definitely shouldn't take longer than 2 seconds. If it takes longer, it fails the test, so everyone knows that there is an issue.

Now, if you combine a) and b) with the Mockito overhead and AssertJ overhead, if our test is "unlucky" and ends up being the first test that needs to warm up the proxies, then we are experiencing flaky tests due to the TimeOut issue.

Even if we solve a) and remove the JVM forking, with the current timeouts, the firstly executed tests are still flaky while there is technically nothing wrong with them.

We tried to brainstorm many ideas, but have come to a conclusion that ideally we need to address the root causes rather than force everyone in the company to apply all kinds of workarounds. The root causes are
a) getting rid of the JVM forking (we are working on that part on our side) and
b) requesting performance improvement on the library side. It was clearly considerably faster when CGLIB was used. So we know that it can be theoretically done.

Some questions we would like to explore:

  1. are there any other, faster proxy libraries that would be sufficient and have better performance?
  2. does it need a proxy in the first place? Or could it be done without them altogether?

Thanks for your thoughts.

@joel-costigliola
Copy link
Member

Thinking out loud, @filiphr suggestion or code generation could work. We would need to do POCs to validate these approaches.

@raphw
Copy link
Contributor

raphw commented Apr 13, 2022

Byte Buddy has a build plugin. In theory, you can preproduce classes and use them. This is what I do with Byte Buddy and Mockito on Graal images what is a rather new feature there, but it requires some setup that is costly.

In general, though, short running programs such as single test execution, and dynamic code generation do never go well together. Ideally, you'd keep a warmed-up VM ready where assertj and Mockito reuse classes.

@scordio
Copy link
Member

scordio commented Oct 24, 2022

@raphw is there any example I could look at in Byte Buddy or Mockito about the build plugin?

@scordio scordio added the theme: soft assertions An issue related to the soft assertions label Oct 24, 2022
@scordio scordio self-assigned this Oct 24, 2022
@raphw
Copy link
Contributor

raphw commented Oct 24, 2022

https://github.com/raphw/byte-buddy/tree/master/byte-buddy-maven-plugin

https://github.com/raphw/byte-buddy/tree/master/byte-buddy-gradle-plugin

@mauricioaniche
Copy link
Contributor

We also bumped into a similar challenge! I wonder if anyone has found a good solution for it! :)

@scordio
Copy link
Member

scordio commented Nov 20, 2023

Unfortunately, we haven't had enough capacity to look into it so far.

I'll add it to the next release scope for a first analysis.

@scordio scordio added the status: pending investigation An issue that needs additional investigation label Nov 20, 2023
@scordio scordio added this to the 3.26.0 milestone Nov 20, 2023
@scordio scordio modified the milestones: 3.26.0, 3.27.0 Apr 9, 2024
@tedyoung
Copy link
Contributor

I'd love to see improvement here because I've had to stop using SoftAssertions in my tests when doing TDD. Even 500ms is too long when the entire set of tests run in under 900ms otherwise. Anything I can do to help?

@scordio
Copy link
Member

scordio commented Jun 22, 2024

Thanks for the offer, @tedyoung!

We still didn't have enough time to try out the proposal at #2563 (comment) and #2563 (comment).

Would you like to give it a shot?

@scordio scordio removed their assignment Jun 22, 2024
@scordio scordio moved this to Backlog in HCP Switzerland 2024 Nov 10, 2024
@scordio scordio modified the milestones: 3.27.0, 3.28.0 Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pending investigation An issue that needs additional investigation theme: soft assertions An issue related to the soft assertions
Projects
None yet
Development

No branches or pull requests

8 participants