Skip to content

Commit 73a861f

Browse files
j-bakerj-baker
authored
Fixes #2720: Use StackWalker on Java 9+ to create Locations (#2723)
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.
1 parent 89698ba commit 73a861f

26 files changed

Lines changed: 634 additions & 157 deletions

‎src/main/java/org/mockito/exceptions/stacktrace/StackTraceCleaner.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,29 @@ public interface StackTraceCleaner {
2525
* @return whether the element should be excluded from cleaned stack trace.
2626
*/
2727
boolean isIn(StackTraceElement candidate);
28+
29+
/**
30+
* It's recommended to override this method in subclasses to avoid potentially costly re-boxing operations.
31+
*/
32+
default boolean isIn(StackFrameMetadata candidate) {
33+
return isIn(
34+
new StackTraceElement(
35+
candidate.getClassName(),
36+
candidate.getMethodName(),
37+
candidate.getFileName(),
38+
candidate.getLineNumber()));
39+
}
40+
41+
/**
42+
* Very similar to the StackFrame class declared on the StackWalker api.
43+
*/
44+
interface StackFrameMetadata {
45+
String getClassName();
46+
47+
String getMethodName();
48+
49+
String getFileName();
50+
51+
int getLineNumber();
52+
}
2853
}

‎src/main/java/org/mockito/internal/MockedConstructionImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
import org.mockito.MockedConstruction;
1313
import org.mockito.exceptions.base.MockitoException;
14-
import org.mockito.internal.debugging.LocationImpl;
14+
import org.mockito.internal.debugging.LocationFactory;
1515
import org.mockito.invocation.Location;
1616
import org.mockito.plugins.MockMaker;
1717

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

2222
private boolean closed;
2323

24-
private final Location location = new LocationImpl();
24+
private final Location location = LocationFactory.create();
2525

2626
protected MockedConstructionImpl(MockMaker.ConstructionMockControl<T> control) {
2727
this.control = control;

‎src/main/java/org/mockito/internal/MockedStaticImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import org.mockito.Mockito;
1818
import org.mockito.exceptions.base.MockitoAssertionError;
1919
import org.mockito.exceptions.base.MockitoException;
20-
import org.mockito.internal.debugging.LocationImpl;
20+
import org.mockito.internal.debugging.LocationFactory;
2121
import org.mockito.internal.listeners.VerificationStartedNotifier;
2222
import org.mockito.internal.progress.MockingProgress;
2323
import org.mockito.internal.stubbing.InvocationContainerImpl;
@@ -35,7 +35,7 @@ public final class MockedStaticImpl<T> implements MockedStatic<T> {
3535

3636
private boolean closed;
3737

38-
private final Location location = new LocationImpl();
38+
private final Location location = LocationFactory.create();
3939

4040
protected MockedStaticImpl(MockMaker.StaticMockControl<T> control) {
4141
this.control = control;

‎src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
import org.mockito.exceptions.base.MockitoException;
5252
import org.mockito.internal.configuration.plugins.Plugins;
5353
import org.mockito.internal.creation.bytebuddy.inject.MockMethodDispatcher;
54-
import org.mockito.internal.debugging.LocationImpl;
54+
import org.mockito.internal.debugging.LocationFactory;
5555
import org.mockito.internal.exceptions.stacktrace.ConditionalStackTraceFilter;
5656
import org.mockito.internal.invocation.RealMethod;
5757
import org.mockito.internal.invocation.SerializableMethod;
@@ -132,11 +132,7 @@ public Callable<?> handle(Object instance, Method origin, Object[] arguments) th
132132
}
133133
return new ReturnValueWrapper(
134134
interceptor.doIntercept(
135-
instance,
136-
origin,
137-
arguments,
138-
realMethod,
139-
new LocationImpl(new Throwable(), true)));
135+
instance, origin, arguments, realMethod, LocationFactory.create(true)));
140136
}
141137

142138
@Override
@@ -154,7 +150,7 @@ public Callable<?> handleStatic(Class<?> type, Method origin, Object[] arguments
154150
origin,
155151
arguments,
156152
new StaticMethodCall(selfCallInfo, type, origin, arguments),
157-
new LocationImpl(new Throwable(), true)));
153+
LocationFactory.create(true)));
158154
}
159155

160156
@Override

‎src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodInterceptor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import net.bytebuddy.implementation.bind.annotation.StubValue;
2323
import net.bytebuddy.implementation.bind.annotation.SuperCall;
2424
import net.bytebuddy.implementation.bind.annotation.This;
25-
import org.mockito.internal.debugging.LocationImpl;
25+
import org.mockito.internal.debugging.LocationFactory;
2626
import org.mockito.internal.invocation.RealMethod;
2727
import org.mockito.invocation.Location;
2828
import org.mockito.invocation.MockHandler;
@@ -53,7 +53,7 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFo
5353

5454
Object doIntercept(Object mock, Method invokedMethod, Object[] arguments, RealMethod realMethod)
5555
throws Throwable {
56-
return doIntercept(mock, invokedMethod, arguments, realMethod, new LocationImpl());
56+
return doIntercept(mock, invokedMethod, arguments, realMethod, LocationFactory.create());
5757
}
5858

5959
Object doIntercept(

‎src/main/java/org/mockito/internal/creation/proxy/ProxyMockMaker.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
package org.mockito.internal.creation.proxy;
66

77
import org.mockito.exceptions.base.MockitoException;
8-
import org.mockito.internal.debugging.LocationImpl;
8+
import org.mockito.internal.debugging.LocationFactory;
99
import org.mockito.internal.invocation.RealMethod;
1010
import org.mockito.internal.util.Platform;
1111
import org.mockito.invocation.MockHandler;
@@ -153,7 +153,12 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
153153
return handler.get()
154154
.handle(
155155
createInvocation(
156-
proxy, method, args, realMethod, settings, new LocationImpl()));
156+
proxy,
157+
method,
158+
args,
159+
realMethod,
160+
settings,
161+
LocationFactory.create()));
157162
}
158163
}
159164

src/main/java/org/mockito/internal/debugging/LocationImpl.java renamed to src/main/java/org/mockito/internal/debugging/Java8LocationImpl.java

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import org.mockito.internal.exceptions.stacktrace.StackTraceFilter;
1010
import org.mockito.invocation.Location;
1111

12-
public class LocationImpl implements Location, Serializable {
12+
class Java8LocationImpl implements Location, Serializable {
1313

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

21-
public LocationImpl() {
22-
this(new Throwable(), false);
23-
}
24-
25-
public LocationImpl(Throwable stackTraceHolder, boolean isInline) {
21+
public Java8LocationImpl(Throwable stackTraceHolder, boolean isInline) {
2622
this(stackTraceFilter, stackTraceHolder, isInline);
2723
}
2824

29-
public LocationImpl(StackTraceFilter stackTraceFilter) {
30-
this(stackTraceFilter, new Throwable(), false);
31-
}
32-
33-
private LocationImpl(
25+
private Java8LocationImpl(
3426
StackTraceFilter stackTraceFilter, Throwable stackTraceHolder, boolean isInline) {
3527
computeStackTraceInformation(stackTraceFilter, stackTraceHolder, isInline);
3628
}

0 commit comments

Comments
 (0)