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

Eagerly compute stackTraceLine in LocationImpl #1683

Merged
merged 10 commits into from Apr 10, 2019
8 changes: 4 additions & 4 deletions build.gradle
Expand Up @@ -7,7 +7,7 @@ buildscript {

dependencies {
classpath 'gradle.plugin.nl.javadude.gradle.plugins:license-gradle-plugin:0.14.0'
classpath 'ru.vyarus:gradle-animalsniffer-plugin:1.4.4' //for 'java-compatibility-check.gradle'
classpath 'ru.vyarus:gradle-animalsniffer-plugin:1.5.0' //for 'java-compatibility-check.gradle'
classpath 'net.ltgt.gradle:gradle-errorprone-plugin:0.6'

//Using buildscript.classpath so that we can resolve shipkit from maven local, during local testing
Expand All @@ -16,7 +16,7 @@ buildscript {
}

plugins {
id 'com.gradle.build-scan' version '1.16'
id 'com.gradle.build-scan' version '2.2.1'
id 'eclipse'
}

Expand Down Expand Up @@ -99,8 +99,8 @@ wrapper {

//Posting Build scans to https://scans.gradle.com
buildScan {
licenseAgreementUrl = 'https://gradle.com/terms-of-service'
licenseAgree = 'yes'
termsOfServiceUrl = 'https://gradle.com/terms-of-service'
termsOfServiceAgree = 'yes'
}

//workaround for #1444, delete when Shipkit bug is fixed
Expand Down
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-5.3.1-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
1 change: 1 addition & 0 deletions settings.gradle
Expand Up @@ -7,6 +7,7 @@ include 'android'
include 'junit-jupiter'
include 'junitJupiterExtensionTest'
include 'module-test'
include 'memory-test'
include 'errorprone'

rootProject.name = 'mockito'
Expand Down
Expand Up @@ -4,6 +4,16 @@
*/
package org.mockito.internal.creation.bytebuddy;

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.Serializable;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.ref.SoftReference;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.concurrent.Callable;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
Expand All @@ -21,19 +31,6 @@
import org.mockito.internal.invocation.mockref.MockWeakReference;
import org.mockito.internal.util.concurrent.WeakConcurrentMap;

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.Serializable;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.ref.SoftReference;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;

public class MockMethodAdvice extends MockMethodDispatcher {

private final WeakConcurrentMap<Object, MockMethodInterceptor> interceptors;
Expand Down Expand Up @@ -105,13 +102,11 @@ public Callable<?> handle(Object instance, Method origin, Object[] arguments) th
} else {
realMethod = new RealMethodCall(selfCallInfo, origin, instance, arguments);
}
Throwable t = new Throwable();
t.setStackTrace(skipInlineMethodElement(t.getStackTrace()));
return new ReturnValueWrapper(interceptor.doIntercept(instance,
origin,
arguments,
realMethod,
new LocationImpl(t)));
new LocationImpl(new Throwable(), true)));
}

@Override
Expand Down Expand Up @@ -222,21 +217,6 @@ private static Object tryInvoke(Method origin, Object instance, Object[] argumen
}
}

// With inline mocking, mocks for concrete classes are not subclassed, so elements of the stubbing methods are not filtered out.
// Therefore, if the method is inlined, skip the element.
private static StackTraceElement[] skipInlineMethodElement(StackTraceElement[] elements) {
List<StackTraceElement> list = new ArrayList<StackTraceElement>(elements.length);
for (int i = 0; i < elements.length; i++) {
StackTraceElement element = elements[i];
list.add(element);
if (element.getClassName().equals(MockMethodAdvice.class.getName()) && element.getMethodName().equals("handle")) {
// If the current element is MockMethodAdvice#handle(), the next is assumed to be an inlined method.
i++;
}
}
return list.toArray(new StackTraceElement[list.size()]);
}

private static class ReturnValueWrapper implements Callable<Object> {

private final Object returned;
Expand Down
56 changes: 31 additions & 25 deletions src/main/java/org/mockito/internal/debugging/LocationImpl.java
Expand Up @@ -11,45 +11,51 @@
public class LocationImpl implements Location, Serializable {

private static final long serialVersionUID = -9054861157390980624L;
//Limit the amount of objects being created, as this class is heavily instantiated:
private static final StackTraceFilter defaultStackTraceFilter = new StackTraceFilter();
// Limit the amount of objects being created, as this class is heavily instantiated:
private static final StackTraceFilter stackTraceFilter = new StackTraceFilter();

private final Throwable stackTraceHolder;
private final StackTraceFilter stackTraceFilter;
private final String sourceFile;
private String stackTraceLine;
private String sourceFile;

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

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

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

private LocationImpl(StackTraceFilter stackTraceFilter, Throwable stackTraceHolder) {
this.stackTraceFilter = stackTraceFilter;
this.stackTraceHolder = stackTraceHolder;
if (stackTraceHolder.getStackTrace() == null || stackTraceHolder.getStackTrace().length == 0) {
//there are corner cases where exception can have a null or empty stack trace
//for example, a custom exception can override getStackTrace() method
this.sourceFile = "<unknown source file>";
} else {
this.sourceFile = stackTraceFilter.findSourceFile(stackTraceHolder.getStackTrace(), "<unknown source file>");
}
private LocationImpl(StackTraceFilter stackTraceFilter, Throwable stackTraceHolder, boolean isInline) {
computeStackTraceInformation(stackTraceFilter, stackTraceHolder, isInline);
}

@Override
public String toString() {
//TODO SF perhaps store the results after invocation?
StackTraceElement[] filtered = stackTraceFilter.filter(stackTraceHolder.getStackTrace(), false);
if (filtered.length == 0) {
return "-> at <<unknown line>>";
return stackTraceLine;
}

/**
* Eagerly compute the stacktrace line from the stackTraceHolder. Storing the Throwable is
* memory-intensive for tests that have large stacktraces and have a lot of invocations on
* mocks.
*/
private void computeStackTraceInformation(
StackTraceFilter stackTraceFilter, Throwable stackTraceHolder, boolean isInline) {
StackTraceElement filtered = stackTraceFilter.filterFirst(stackTraceHolder, isInline);

// there are corner cases where exception can have a null or empty stack trace
// for example, a custom exception can override getStackTrace() method
if (filtered == null) {
this.stackTraceLine = "-> at <<unknown line>>";
this.sourceFile = "<unknown source file>";
} else {
this.stackTraceLine = "-> at " + filtered.toString();
this.sourceFile = filtered.getFileName();
}
return "-> at " + filtered[0].toString();
}

@Override
Expand Down
Expand Up @@ -5,6 +5,7 @@

package org.mockito.internal.exceptions.stacktrace;

import java.lang.reflect.Method;
import org.mockito.exceptions.stacktrace.StackTraceCleaner;
import org.mockito.internal.configuration.plugins.Plugins;

Expand All @@ -19,6 +20,24 @@ public class StackTraceFilter implements Serializable {
private static final StackTraceCleaner CLEANER =
Plugins.getStackTraceCleanerProvider().getStackTraceCleaner(new DefaultStackTraceCleaner());

private static Object JAVA_LANG_ACCESS;
private static Method GET_STACK_TRACE_ELEMENT;

static {
try {
JAVA_LANG_ACCESS =
Class.forName("sun.misc.SharedSecrets")
.getMethod("getJavaLangAccess")
.invoke(null);
GET_STACK_TRACE_ELEMENT =
Class.forName("sun.misc.JavaLangAccess")
.getMethod("getStackTraceElement", Throwable.class, int.class);
} catch (Exception ignored) {
// Use the slow computational path for filtering stacktraces if fast path does not exist
// in JVM
}
}

/**
* Example how the filter works (+/- means good/bad):
* [a+, b+, c-, d+, e+, f-, g+] -> [a+, b+, d+, e+, g+]
Expand All @@ -38,6 +57,67 @@ public StackTraceElement[] filter(StackTraceElement[] target, boolean keepTop) {
return filtered.toArray(result);
}

/**
* This filtering strategy makes use of a fast-path computation to retrieve stackTraceElements
* from a Stacktrace of a Throwable. It does so, by taking advantage of {@link
* sun.misc.SharedSecrets} and {@link sun.misc.JavaLangAccess}.
*
* <p>The {@link sun.misc.SharedSecrets} provides a method to obtain an instance of an {@link
* sun.misc.JavaLangAccess}. The latter class has a method to fast-path into {@link
* Throwable#getStackTrace()} and retrieve a single {@link StackTraceElement}. This prevents the
* JVM from having to generate a full stacktrace, which could potentially be expensive if
* stacktraces become very large.
*
* @param target The throwable target to find the first {@link StackTraceElement} that should
* not be filtered out per {@link StackTraceFilter#CLEANER}.
* @return The first {@link StackTraceElement} outside of the {@link StackTraceFilter#CLEANER}
*/
public StackTraceElement filterFirst(Throwable target, boolean isInline) {
boolean shouldSkip = isInline;

if (GET_STACK_TRACE_ELEMENT != null) {
int i = 0;

// The assumption here is that the CLEANER filter will not filter out every single
// element. However, since we don't want to compute the full length of the stacktrace,
// we don't know the upper boundary. Therefore, simply increment the counter and go as
// far as we have to go, assuming that we get there. If, in the rare occassion, we
// don't, we fall back to the old slow path.
while (true) {
try {
StackTraceElement stackTraceElement =
(StackTraceElement)
GET_STACK_TRACE_ELEMENT.invoke(JAVA_LANG_ACCESS, target, i);

if (CLEANER.isIn(stackTraceElement)) {
if (shouldSkip) {
shouldSkip = false;
} else {
return stackTraceElement;
}
}
} catch (Exception e) {
// Fall back to slow path
break;
}
i++;
}
}

// If we can't use the fast path of retrieving stackTraceElements, use the slow path by
// iterating over the actual stacktrace
for (StackTraceElement stackTraceElement : target.getStackTrace()) {
if (CLEANER.isIn(stackTraceElement)) {
if (shouldSkip) {
shouldSkip = false;
} else {
return stackTraceElement;
}
}
}
return null;
}

/**
* Finds the source file of the target stack trace.
* Returns the default value if source file cannot be found.
Expand Down
Expand Up @@ -31,6 +31,11 @@ public void shouldBeSafeInCaseForSomeReasonFilteredStackTraceIsEmpty() {
public StackTraceElement[] filter(StackTraceElement[] target, boolean keepTop) {
return new StackTraceElement[0];
}

@Override
public StackTraceElement filterFirst(Throwable target, boolean isInline) {
return null;
}
};

//when
Expand Down
19 changes: 19 additions & 0 deletions subprojects/memory-test/memory-test.gradle
@@ -0,0 +1,19 @@
plugins {
id 'java'
}
description = "Test suite memory usage of Mockito"

apply from: "$rootDir/gradle/dependencies.gradle"

dependencies {
compile project.rootProject
testCompile libraries.junit4
testCompile libraries.assertj
}

tasks.javadoc.enabled = false

test {
maxHeapSize = "128m"
jvmArgs = ["-XX:MaxPermSize=128m"]
}
@@ -0,0 +1,60 @@
/*
* Copyright (c) 2016 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockito.memorytest;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.junit.Assume;
import org.junit.Test;

public class ShouldNotStarveMemoryOnLargeStackTraceInvocationsTest {

private static final int STACK_TRACE_DEPTH = 1000;
private static final int INVOCATIONS_ON_STACK_TRACE_LEVEL = 100;

private static boolean supported = false;

static {
try {
Class.forName("sun.misc.SharedSecrets")
.getMethod("getJavaLangAccess")
.invoke(null);
Class.forName("sun.misc.JavaLangAccess")
.getMethod("getStackTraceElement", Throwable.class, int.class);

supported = true;
} catch (Exception ignored) {
}
}

@Test
public void large_stack_trace_invocations_should_not_starve_memory() {
Assume.assumeTrue(supported);
Dummy mock = mock(Dummy.class);

when(mock.calculate(anyInt())).thenReturn(42);

assertThat(performComputationForDepth(mock, STACK_TRACE_DEPTH)).isEqualTo(42);
}

private int performComputationForDepth(Dummy mock, int i) {
if (i > 0) {
for (int j = 0; j < INVOCATIONS_ON_STACK_TRACE_LEVEL; j++) {
mock.calculate(j);
}

return mock.calculate(performComputationForDepth(mock, i - 1));
}

return 1;
}

interface Dummy {
int calculate(int fib);
}
}