-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
LocationImpl adds performance overheads due to instantiating a stack trace #2720
Comments
mockito/src/main/java/org/mockito/internal/exceptions/stacktrace/StackTraceFilter.java Line 75 in 95b43e5
Maybe you can use these to fix the performance problems? |
Yes, the method currently used relies on JavaLangAccess containing a openjdk/jdk@eb2c6c5#diff-9908a97582edc388983be46caac4de91a4b958df46d86b9bd739f577a55c01b0L105 |
In terms of memory allocations, this reduces the overall memory allocations of creating a location by an order of magnitude in Java 9, and as compared to Java 8. The implementation is somewhat involved due to these desires: - Minimize the amount of work done if the Location is never used. This is done by not converting the StackFrame into a StackTraceElement, instead wrapping in an intermediate state. The StackTraceElement conversion will only occur (internally) if the .getFileName() method is called. - Ensure the implementation is still Serializable. This is ensured with a .writeReplace method. - Minimize the number of allocations, which is basically an exercise in lambda caching. - Ensuring the old mechanism still works on Java 8. Presently on Java 9+, on a stack depth of 1000 the old mechanism will allocate 40kB of RAM per call. The new one will allocate 1.5kB of RAM per call, which is a huge improvement. This is still sadly not the 'close-to-no-overhead' solution I was looking for. I therefore also added a system property that can be used to fully disable Location creation. I'm aware that this is likely not the right approach given Mockito has plugins and settings - mostly looking for guidance here given I'm not sure what would be idiomatic here.
In terms of memory allocations, this reduces the overall memory allocations of creating a location by an order of magnitude in Java 9, and as compared to Java 8. The implementation is somewhat involved due to these desires: - Minimize the amount of work done if the Location is never used. This is done by not converting the StackFrame into a StackTraceElement, instead wrapping in an intermediate state. The StackTraceElement conversion will only occur (internally) if the .getFileName() method is called. - Ensure the implementation is still Serializable. This is ensured with a .writeReplace method. - Minimize the number of allocations, which is basically an exercise in lambda caching. - Ensuring the old mechanism still works on Java 8. Presently on Java 9+, on a stack depth of 1000 the old mechanism will allocate 40kB of RAM per call. The new one will allocate 1.5kB of RAM per call, which is a huge improvement. This is still sadly not the 'close-to-no-overhead' solution I was looking for. I therefore also added a system property that can be used to fully disable Location creation. I'm aware that this is likely not the right approach given Mockito has plugins and settings - mostly looking for guidance here given I'm not sure what would be idiomatic here.
In terms of memory allocations, this reduces the overall memory allocations of creating a location by an order of magnitude in Java 9, and as compared to Java 8. The implementation is somewhat involved due to these desires: - Minimize the amount of work done if the Location is never used. This is done by not converting the StackFrame into a StackTraceElement, instead wrapping in an intermediate state. The StackTraceElement conversion will only occur (internally) if the .getFileName() method is called. - Ensure the implementation is still Serializable. This is ensured with a .writeReplace method. - Minimize the number of allocations, which is basically an exercise in lambda caching. - Ensuring the old mechanism still works on Java 8. Presently on Java 9+, on a stack depth of 1000 the old mechanism will allocate 40kB of RAM per call. The new one will allocate 1.5kB of RAM per call, which is a huge improvement. This is still sadly not the 'close-to-no-overhead' solution I was looking for. I therefore also added a system property that can be used to fully disable Location creation. I'm aware that this is likely not the right approach given Mockito has plugins and settings - mostly looking for guidance here given I'm not sure what would be idiomatic here.
I'm trying to use Mockito in a test of a Java class I didn't write. It takes a dependency which I am stubbing out using Mockito. Skipping all of the business logic, the code ends up being rather similar to this:
This is in Mockito 4.6.1. This is extremely slow, and looking at Mockito this seems to be a problem for two reasons.
Firstly, when a mock invocation occurs, LocationImpl is instantiated. This instantiates a Throwable, which constructs a stack trace, error message, etc (which is then discarded). In my test, Mockito was generating 70k exceptions per second.
The second slowdown that seemed to be affecting me comes from TypeSafeMatching.getArgumentType. This iterates through all of the methods on the argument matcher class to find the appropriate matches method, seemingly also for each method call. By performing this operation, a lot of JVM internal datastructures get copied - it calls Class.getMethods, which copies all the methods on the class. This method could probably be cached on a per-class basis.
This dramatically affected my application's allocation rate; Mockito was leading to approx 3.4GiB/sec allocations. In the end, I rewrote my mock using ByteBuddy proxies directly. It took the test runtime from 30 seconds down to 1, with the number of exceptions being created per second going down from 70k to ~0.
I know that Mockito isn't meant to be a Netty-like high performance library, but wanted to flag these two codepaths as affecting every mock call.
The text was updated successfully, but these errors were encountered: