From b0364f3c42050a3fc3ef60883ab95888bad93578 Mon Sep 17 00:00:00 2001 From: rob Date: Tue, 21 Feb 2023 14:21:18 +1300 Subject: [PATCH] Use StackWalker for ProfileLocation and CallOrigin --- .../main/java/io/ebean/bean/CallStack.java | 14 +++- .../java/io/ebean/util/StackWalkFilter.java | 28 ++++++++ .../server/core/DefaultCallOriginFactory.java | 65 ++++--------------- .../server/core/NoopCallOriginFactory.java | 2 +- .../server/profile/DProfileLocation.java | 20 +++--- .../core/HelpDefaultCallOriginFactory.java | 11 ++++ .../profile/BasicProfileLocationTest.java | 17 +---- .../server/TestDefaultCallOriginFactory.java | 25 +++++++ .../tests/profile/ProfileLocationTest.java | 9 ++- 9 files changed, 107 insertions(+), 84 deletions(-) create mode 100644 ebean-api/src/main/java/io/ebean/util/StackWalkFilter.java create mode 100644 ebean-core/src/test/java/io/ebeaninternal/server/core/HelpDefaultCallOriginFactory.java create mode 100644 ebean-core/src/test/java/org/tests/server/TestDefaultCallOriginFactory.java diff --git a/ebean-api/src/main/java/io/ebean/bean/CallStack.java b/ebean-api/src/main/java/io/ebean/bean/CallStack.java index 2d2a5ffe5..934f53321 100644 --- a/ebean-api/src/main/java/io/ebean/bean/CallStack.java +++ b/ebean-api/src/main/java/io/ebean/bean/CallStack.java @@ -2,6 +2,7 @@ package io.ebean.bean; import java.io.Serializable; import java.util.Arrays; +import java.util.List; import static io.ebean.util.EncodeB64.enc; @@ -27,19 +28,26 @@ public final class CallStack implements Serializable, CallOrigin { private final String zeroHash; private final String pathHash; - private final StackTraceElement[] callStack; + private final Object[] callStack; private final int hc; - public CallStack(StackTraceElement[] callStack, int zeroHash, int pathHash) { + public CallStack(Object[] callStack, int zeroHash, int pathHash) { this.callStack = callStack; + this.hc = computeHashCode(); this.zeroHash = enc(zeroHash); this.pathHash = enc(pathHash); + } + + public CallStack(List frames) { + this.callStack = frames.toArray(new Object[0]); this.hc = computeHashCode(); + this.zeroHash = enc(callStack[0].hashCode()); + this.pathHash = enc(hc); } private int computeHashCode() { int hc = 0; - for (StackTraceElement element : callStack) { + for (Object element : callStack) { hc = 92821 * hc + element.hashCode(); } return hc; diff --git a/ebean-api/src/main/java/io/ebean/util/StackWalkFilter.java b/ebean-api/src/main/java/io/ebean/util/StackWalkFilter.java new file mode 100644 index 000000000..a68df25d4 --- /dev/null +++ b/ebean-api/src/main/java/io/ebean/util/StackWalkFilter.java @@ -0,0 +1,28 @@ +package io.ebean.util; + +import java.util.function.Predicate; + +/** + * Provides a stack filter that excludes ebean and jdk code. + */ +public final class StackWalkFilter { + + private static final Filter FILTER = new Filter(); + + /** + * Return a stack filter that excludes ebean and jdk code. + */ + public static Predicate filter() { + return FILTER; + } + + private static class Filter implements Predicate { + + @Override + public boolean test(StackWalker.StackFrame stackFrame) { + return !stackFrame.getClassName().startsWith("io.ebean") + && !stackFrame.getClassName().startsWith("jdk.") + && !stackFrame.getMethodName().startsWith("_ebean_"); + } + } +} diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultCallOriginFactory.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultCallOriginFactory.java index 233a2a920..e462ce303 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultCallOriginFactory.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultCallOriginFactory.java @@ -2,17 +2,17 @@ package io.ebeaninternal.server.core; import io.ebean.bean.CallOrigin; import io.ebean.bean.CallStack; +import io.ebean.util.StackWalkFilter; import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Default CallStackFactory where the Hash function for StackTraceElement includes the line number. */ -public final class DefaultCallOriginFactory implements CallOriginFactory { - - private static final int IGNORE_LEADING_ELEMENTS = 5; - - private static final String IO_EBEAN = "io.ebean"; +final class DefaultCallOriginFactory implements CallOriginFactory { private final int maxCallStack; @@ -22,57 +22,18 @@ public final class DefaultCallOriginFactory implements CallOriginFactory { @Override public CallOrigin createCallOrigin() { - StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); - - // ignore the first 6 as they are always avaje stack elements - int startIndex = IGNORE_LEADING_ELEMENTS; - - // find the first non-avaje stackElement - for (; startIndex < stackTrace.length; startIndex++) { - if (!ignore(stackTrace[startIndex])) { - break; - } - } - - int stackLength = stackTrace.length - startIndex; - if (stackLength > maxCallStack) { - // maximum of maxCallStack stackTrace elements - stackLength = maxCallStack; - } - - // create the 'interesting' part of the stackTrace - StackTraceElement[] finalTrace = new StackTraceElement[stackLength]; - System.arraycopy(stackTrace, startIndex, finalTrace, 0, stackLength); - - if (stackLength < 1) { + final var frames = StackWalker.getInstance().walk(this::filter); + if (frames.isEmpty()) { // this should not really happen - throw new RuntimeException("StackTraceElement size 0? stack: " + Arrays.toString(stackTrace)); + throw new RuntimeException("stackFrames filtered to empty for stack: " + Arrays.toString(Thread.currentThread().getStackTrace())); } - - return createCallStack(finalTrace); + return new CallStack(frames); } - private boolean ignore(StackTraceElement element) { - if (element.getClassName().startsWith(IO_EBEAN)) { - return true; - } - return element.getMethodName().startsWith("_ebean_"); - } - - private CallOrigin createCallStack(StackTraceElement[] finalTrace) { - return new CallStack(finalTrace, finalTrace[0].hashCode(), pathHash(finalTrace)); - } - - /** - * Return the hash code for the path excluding the first element. - */ - private int pathHash(StackTraceElement[] callStack) { - - int hc = 0; - for (int i = 1; i < callStack.length; i++) { - hc = 92821 * hc + callStack[i].hashCode(); - } - return hc; + private List filter(Stream frames) { + return frames.filter(StackWalkFilter.filter()) + .limit(maxCallStack) + .collect(Collectors.toList()); } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/NoopCallOriginFactory.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/NoopCallOriginFactory.java index a95e507ea..b8624ca8b 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/NoopCallOriginFactory.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/NoopCallOriginFactory.java @@ -10,7 +10,7 @@ final class NoopCallOriginFactory implements CallOriginFactory { private static final StackTraceElement E0 = new StackTraceElement("none", "none", "none", 0); - private final CallOrigin COMMON = new CallStack(new StackTraceElement[]{E0}, 0, 0); + private final CallOrigin COMMON = new CallStack(new Object[]{E0}, 0, 0); @Override public CallOrigin createCallOrigin() { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DProfileLocation.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DProfileLocation.java index 684cd1a00..ed56cf89f 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DProfileLocation.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DProfileLocation.java @@ -1,13 +1,15 @@ package io.ebeaninternal.server.profile; import io.ebean.ProfileLocation; +import io.ebean.util.StackWalkFilter; + +import java.util.stream.Stream; /** * Default profile location that uses stack trace. */ class DProfileLocation implements ProfileLocation { - private static final String IO_EBEAN = "io.ebean"; private static final String UNKNOWN = "unknown"; private String fullLocation; @@ -88,14 +90,14 @@ class DProfileLocation implements ProfileLocation { } private String create() { - // relatively expensive but we only do it once per profile location - StackTraceElement[] trace = Thread.currentThread().getStackTrace(); - for (int i = 3; i < trace.length; i++) { - if (!trace[i].getClassName().startsWith(IO_EBEAN)) { - return withLineNumber(trace[i].toString()); - } - } - return UNKNOWN; + return StackWalker.getInstance().walk(this::filter); + } + + private String filter(Stream frames) { + return frames.filter(StackWalkFilter.filter()) + .findFirst() + .map(line -> withLineNumber(line.toString())) + .orElse(UNKNOWN); } private String withLineNumber(String traceLine) { diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/core/HelpDefaultCallOriginFactory.java b/ebean-core/src/test/java/io/ebeaninternal/server/core/HelpDefaultCallOriginFactory.java new file mode 100644 index 000000000..17de45590 --- /dev/null +++ b/ebean-core/src/test/java/io/ebeaninternal/server/core/HelpDefaultCallOriginFactory.java @@ -0,0 +1,11 @@ +package io.ebeaninternal.server.core; + +/** + * Make DefaultCallOriginFactory accessible to tests. + */ +public class HelpDefaultCallOriginFactory { + + public static CallOriginFactory create(int maxStack) { + return new DefaultCallOriginFactory(maxStack); + } +} diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/profile/BasicProfileLocationTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/profile/BasicProfileLocationTest.java index f93e12979..7e89b08a8 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/profile/BasicProfileLocationTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/profile/BasicProfileLocationTest.java @@ -54,21 +54,10 @@ class BasicProfileLocationTest { void obtain() { DProfileLocation loc = new DTimedProfileLocation(12, "foo", MetricFactory.get().createTimedMetric("junk")); - String javaVersion = System.getProperty("java.version"); assertThat(loc.obtain()).isTrue(); - if (javaVersion.startsWith("1.8")) { - assertThat(loc.fullLocation()).endsWith("invoke0(Native Method:12)"); - assertThat(loc.location()).isEqualTo("sun.reflect.NativeMethodAccessorImpl.invoke0"); - assertThat(loc.label()).isEqualTo("NativeMethodAccessorImpl.invoke0"); - } else if (javaVersion.startsWith("18") || javaVersion.startsWith("19") || javaVersion.startsWith("20")){ - assertThat(loc.fullLocation()).endsWith("jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)"); - assertThat(loc.location()).isEqualTo("java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke"); - assertThat(loc.label()).isEqualTo("DirectMethodHandleAccessor.invoke"); - } else if (javaVersion.startsWith("11") || javaVersion.startsWith("17")) { - assertThat(loc.fullLocation()).endsWith("jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method:12)"); - assertThat(loc.location()).isEqualTo("java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0"); - assertThat(loc.label()).isEqualTo("NativeMethodAccessorImpl.invoke0"); - } + assertThat(loc.fullLocation()).endsWith("org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)"); + assertThat(loc.location()).isEqualTo("org.junit.platform.commons.util.ReflectionUtils.invokeMethod"); + assertThat(loc.label()).isEqualTo("ReflectionUtils.invokeMethod"); } @Test diff --git a/ebean-core/src/test/java/org/tests/server/TestDefaultCallOriginFactory.java b/ebean-core/src/test/java/org/tests/server/TestDefaultCallOriginFactory.java new file mode 100644 index 000000000..16534eb9b --- /dev/null +++ b/ebean-core/src/test/java/org/tests/server/TestDefaultCallOriginFactory.java @@ -0,0 +1,25 @@ +package org.tests.server; + +import io.ebean.bean.CallOrigin; +import io.ebeaninternal.server.core.CallOriginFactory; +import io.ebeaninternal.server.core.HelpDefaultCallOriginFactory; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class TestDefaultCallOriginFactory { + + private final CallOriginFactory factory = HelpDefaultCallOriginFactory.create(2); + + @Test + void createCallOrigin() { + CallOrigin callOrigin = inner(); + String topElement = callOrigin.getTopElement(); + assertThat(topElement).contains("org.tests.server.TestDefaultCallOriginFactory.inner(TestDefaultCallOriginFactory.java:23)"); + assertThat(callOrigin.getFullDescription()).contains("TestDefaultCallOriginFactory.java:16"); + } + + private CallOrigin inner() { + return factory.createCallOrigin(); + } +} diff --git a/ebean-test/src/test/java/org/tests/profile/ProfileLocationTest.java b/ebean-test/src/test/java/org/tests/profile/ProfileLocationTest.java index b89c5e448..b14b35963 100644 --- a/ebean-test/src/test/java/org/tests/profile/ProfileLocationTest.java +++ b/ebean-test/src/test/java/org/tests/profile/ProfileLocationTest.java @@ -5,7 +5,7 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; -public class ProfileLocationTest { +class ProfileLocationTest { private static final ProfileLocation loc = ProfileLocation.create(12, "foo"); private static final ProfileLocation locB = ProfileLocation.create(); @@ -17,7 +17,7 @@ public class ProfileLocationTest { } @Test - public void test_obtain() { + void test_obtain() { assertThat(doIt()).isTrue(); assertThat(loc.fullLocation()).isEqualTo("org.tests.profile.ProfileLocationTest.doIt(ProfileLocationTest.java:16)"); assertThat(loc.location()).isEqualTo("org.tests.profile.ProfileLocationTest.doIt"); @@ -30,13 +30,12 @@ public class ProfileLocationTest { } @Test - public void test_add() { + void test_add() { loc.add(100); } @Test - public void test_constructor() { - + void test_constructor() { Other other = new Other(); other.hashCode();