Skip to content

Commit

Permalink
Fixes #2720: Use StackWalker on Java 9+ to create Locations (#2723)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
j-baker committed Aug 13, 2022
1 parent 89698ba commit 73a861f
Show file tree
Hide file tree
Showing 26 changed files with 634 additions and 157 deletions.
Expand Up @@ -25,4 +25,29 @@ public interface StackTraceCleaner {
* @return whether the element should be excluded from cleaned stack trace.
*/
boolean isIn(StackTraceElement candidate);

/**
* It's recommended to override this method in subclasses to avoid potentially costly re-boxing operations.
*/
default boolean isIn(StackFrameMetadata candidate) {
return isIn(
new StackTraceElement(
candidate.getClassName(),
candidate.getMethodName(),
candidate.getFileName(),
candidate.getLineNumber()));
}

/**
* Very similar to the StackFrame class declared on the StackWalker api.
*/
interface StackFrameMetadata {
String getClassName();

String getMethodName();

String getFileName();

int getLineNumber();
}
}
Expand Up @@ -11,7 +11,7 @@

import org.mockito.MockedConstruction;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.debugging.LocationImpl;
import org.mockito.internal.debugging.LocationFactory;
import org.mockito.invocation.Location;
import org.mockito.plugins.MockMaker;

Expand All @@ -21,7 +21,7 @@ public final class MockedConstructionImpl<T> implements MockedConstruction<T> {

private boolean closed;

private final Location location = new LocationImpl();
private final Location location = LocationFactory.create();

protected MockedConstructionImpl(MockMaker.ConstructionMockControl<T> control) {
this.control = control;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/mockito/internal/MockedStaticImpl.java
Expand Up @@ -17,7 +17,7 @@
import org.mockito.Mockito;
import org.mockito.exceptions.base.MockitoAssertionError;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.debugging.LocationImpl;
import org.mockito.internal.debugging.LocationFactory;
import org.mockito.internal.listeners.VerificationStartedNotifier;
import org.mockito.internal.progress.MockingProgress;
import org.mockito.internal.stubbing.InvocationContainerImpl;
Expand All @@ -35,7 +35,7 @@ public final class MockedStaticImpl<T> implements MockedStatic<T> {

private boolean closed;

private final Location location = new LocationImpl();
private final Location location = LocationFactory.create();

protected MockedStaticImpl(MockMaker.StaticMockControl<T> control) {
this.control = control;
Expand Down
Expand Up @@ -51,7 +51,7 @@
import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.configuration.plugins.Plugins;
import org.mockito.internal.creation.bytebuddy.inject.MockMethodDispatcher;
import org.mockito.internal.debugging.LocationImpl;
import org.mockito.internal.debugging.LocationFactory;
import org.mockito.internal.exceptions.stacktrace.ConditionalStackTraceFilter;
import org.mockito.internal.invocation.RealMethod;
import org.mockito.internal.invocation.SerializableMethod;
Expand Down Expand Up @@ -132,11 +132,7 @@ public Callable<?> handle(Object instance, Method origin, Object[] arguments) th
}
return new ReturnValueWrapper(
interceptor.doIntercept(
instance,
origin,
arguments,
realMethod,
new LocationImpl(new Throwable(), true)));
instance, origin, arguments, realMethod, LocationFactory.create(true)));
}

@Override
Expand All @@ -154,7 +150,7 @@ public Callable<?> handleStatic(Class<?> type, Method origin, Object[] arguments
origin,
arguments,
new StaticMethodCall(selfCallInfo, type, origin, arguments),
new LocationImpl(new Throwable(), true)));
LocationFactory.create(true)));
}

@Override
Expand Down
Expand Up @@ -22,7 +22,7 @@
import net.bytebuddy.implementation.bind.annotation.StubValue;
import net.bytebuddy.implementation.bind.annotation.SuperCall;
import net.bytebuddy.implementation.bind.annotation.This;
import org.mockito.internal.debugging.LocationImpl;
import org.mockito.internal.debugging.LocationFactory;
import org.mockito.internal.invocation.RealMethod;
import org.mockito.invocation.Location;
import org.mockito.invocation.MockHandler;
Expand Down Expand Up @@ -53,7 +53,7 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFo

Object doIntercept(Object mock, Method invokedMethod, Object[] arguments, RealMethod realMethod)
throws Throwable {
return doIntercept(mock, invokedMethod, arguments, realMethod, new LocationImpl());
return doIntercept(mock, invokedMethod, arguments, realMethod, LocationFactory.create());
}

Object doIntercept(
Expand Down
Expand Up @@ -5,7 +5,7 @@
package org.mockito.internal.creation.proxy;

import org.mockito.exceptions.base.MockitoException;
import org.mockito.internal.debugging.LocationImpl;
import org.mockito.internal.debugging.LocationFactory;
import org.mockito.internal.invocation.RealMethod;
import org.mockito.internal.util.Platform;
import org.mockito.invocation.MockHandler;
Expand Down Expand Up @@ -153,7 +153,12 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
return handler.get()
.handle(
createInvocation(
proxy, method, args, realMethod, settings, new LocationImpl()));
proxy,
method,
args,
realMethod,
settings,
LocationFactory.create()));
}
}

Expand Down
Expand Up @@ -9,7 +9,7 @@
import org.mockito.internal.exceptions.stacktrace.StackTraceFilter;
import org.mockito.invocation.Location;

public class LocationImpl implements Location, Serializable {
class Java8LocationImpl implements Location, Serializable {

private static final long serialVersionUID = -9054861157390980624L;
// Limit the amount of objects being created, as this class is heavily instantiated:
Expand All @@ -18,19 +18,11 @@ public class LocationImpl implements Location, Serializable {
private String stackTraceLine;
private String sourceFile;

public LocationImpl() {
this(new Throwable(), false);
}

public LocationImpl(Throwable stackTraceHolder, boolean isInline) {
public Java8LocationImpl(Throwable stackTraceHolder, boolean isInline) {
this(stackTraceFilter, stackTraceHolder, isInline);
}

public LocationImpl(StackTraceFilter stackTraceFilter) {
this(stackTraceFilter, new Throwable(), false);
}

private LocationImpl(
private Java8LocationImpl(
StackTraceFilter stackTraceFilter, Throwable stackTraceHolder, boolean isInline) {
computeStackTraceInformation(stackTraceFilter, stackTraceHolder, isInline);
}
Expand Down

0 comments on commit 73a861f

Please sign in to comment.