[POOL-320]: Use more efficient stack walking mechanisms for usage tracking git-svn-id: https://svn.apache.org/repos/asf/commons/proper/pool/trunk@1785520 13f79535-47bb-0310-9956-ffa450edef68
Project: http://git-wip-us.apache.org/repos/asf/commons-pool/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-pool/commit/3994baf0 Tree: http://git-wip-us.apache.org/repos/asf/commons-pool/tree/3994baf0 Diff: http://git-wip-us.apache.org/repos/asf/commons-pool/diff/3994baf0 Branch: refs/heads/master Commit: 3994baf0f3ce59b73bd36e869320275d757d1884 Parents: d00d176 Author: Matt Sicker <mattsic...@apache.org> Authored: Sat Mar 4 20:18:19 2017 +0000 Committer: Matt Sicker <mattsic...@apache.org> Committed: Sat Mar 4 20:18:19 2017 +0000 ---------------------------------------------------------------------- src/changes/changes.xml | 3 + .../apache/commons/pool2/impl/CallStack.java | 54 ++++++++++ .../commons/pool2/impl/CallStackUtils.java | 62 +++++++++++ .../commons/pool2/impl/DefaultPooledObject.java | 71 +++--------- .../pool2/impl/SecurityManagerCallStack.java | 107 +++++++++++++++++++ .../commons/pool2/impl/ThrowableCallStack.java | 76 +++++++++++++ .../commons/pool2/impl/CallStackTest.java | 63 +++++++++++ .../pool2/impl/TestDefaultPooledObjectInfo.java | 17 ++- 8 files changed, 383 insertions(+), 70 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7409dc3..c710cd5 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -83,6 +83,9 @@ The <action> type attribute can be add,update,fix,remove. Ensure that a call to GKOP preparePool() takes account of other threads that might create objects concurrently, particularly the Evictor. </action> + <action dev="mattsicker" issue="POOL-320" type="add"> + Use more efficient stack walking mechanisms for usage tracking when possible. + </action> </release> <release version="2.4.2" date="2015-08-01" description= "This is a patch release, including bug fixes only."> http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/main/java/org/apache/commons/pool2/impl/CallStack.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/pool2/impl/CallStack.java b/src/main/java/org/apache/commons/pool2/impl/CallStack.java new file mode 100644 index 0000000..9986553 --- /dev/null +++ b/src/main/java/org/apache/commons/pool2/impl/CallStack.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.commons.pool2.impl; + +import org.apache.commons.pool2.PooledObject; +import org.apache.commons.pool2.UsageTracking; + +import java.io.PrintWriter; + +/** + * Strategy for obtaining and printing the current call stack. This is primarily useful for + * {@linkplain UsageTracking usage tracking} so that different JVMs and configurations can use more efficient strategies + * for obtaining the current call stack. + * + * @see CallStackUtils + * @since 2.4.3 + */ +public interface CallStack { + + /** + * Prints the current stack trace if available to a PrintWriter. The format is undefined and is primarily useful + * for debugging issues with {@link PooledObject} usage in user code. + * + * @param writer a PrintWriter to write the curren stack trace to if available + * @return true if a stack trace was available to print or false if nothing was printed + */ + boolean printStackTrace(final PrintWriter writer); + + /** + * Takes a snapshot of the current call stack. Subsequent calls to {@link #printStackTrace(PrintWriter)} will print + * out that stack trace until it is {@linkplain #clear() cleared}. + */ + void fillInStackTrace(); + + /** + * Clears the current stack trace snapshot. Subsequent calls to {@link #printStackTrace(PrintWriter)} will be + * no-ops until another call to {@link #fillInStackTrace()}. + */ + void clear(); +} http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java b/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java new file mode 100644 index 0000000..7700651 --- /dev/null +++ b/src/main/java/org/apache/commons/pool2/impl/CallStackUtils.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.commons.pool2.impl; + +import java.security.AccessControlException; + +/** + * Utility methods for {@link CallStack}. + * + * @since 2.4.3 + */ +public final class CallStackUtils { + + private static final boolean CAN_CREATE_SECURITY_MANAGER; + + static { + CAN_CREATE_SECURITY_MANAGER = canCreateSecurityManager(); + } + + private static boolean canCreateSecurityManager() { + SecurityManager manager = System.getSecurityManager(); + if (manager == null) { + return true; + } + try { + manager.checkPermission(new RuntimePermission("createSecurityManager")); + return true; + } catch (final AccessControlException ignored) { + return false; + } + } + + /** + * Constructs a new {@link CallStack} using the fastest allowed strategy. + * + * @param messageFormat message (or format) to print first in stack traces + * @param useTimestamp if true, interpret message as a SimpleDateFormat and print the created timestamp; otherwise, + * print message format literally + * @return a new CallStack + */ + public static CallStack newCallStack(final String messageFormat, final boolean useTimestamp) { + return CAN_CREATE_SECURITY_MANAGER ? new SecurityManagerCallStack(messageFormat, useTimestamp) + : new ThrowableCallStack(messageFormat, useTimestamp); + } + + private CallStackUtils() { + } +} http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java index 3caa40c..39ad54b 100644 --- a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java +++ b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java @@ -16,15 +16,13 @@ */ package org.apache.commons.pool2.impl; -import java.io.PrintWriter; -import java.text.SimpleDateFormat; -import java.util.Date; -import java.util.Deque; - import org.apache.commons.pool2.PooledObject; import org.apache.commons.pool2.PooledObjectState; import org.apache.commons.pool2.TrackedUse; +import java.io.PrintWriter; +import java.util.Deque; + /** * This wrapper is used to track the additional information, such as state, for * the pooled objects. @@ -46,8 +44,10 @@ public class DefaultPooledObject<T> implements PooledObject<T> { private volatile long lastUseTime = createTime; private volatile long lastReturnTime = createTime; private volatile boolean logAbandoned = false; - private volatile Exception borrowedBy = null; - private volatile Exception usedBy = null; + private final CallStack borrowedBy = CallStackUtils.newCallStack("'Pooled object created' " + + "yyyy-MM-dd HH:mm:ss Z 'by the following code has not been returned to the pool:'", true); + private final CallStack usedBy = CallStackUtils.newCallStack("The last code to use this object was:", + false); private volatile long borrowedCount = 0; /** @@ -193,7 +193,7 @@ public class DefaultPooledObject<T> implements PooledObject<T> { lastUseTime = lastBorrowTime; borrowedCount++; if (logAbandoned) { - borrowedBy = new AbandonedObjectCreatedException(); + borrowedBy.fillInStackTrace(); } return true; } else if (state == PooledObjectState.EVICTION) { @@ -218,7 +218,7 @@ public class DefaultPooledObject<T> implements PooledObject<T> { state == PooledObjectState.RETURNING) { state = PooledObjectState.IDLE; lastReturnTime = System.currentTimeMillis(); - borrowedBy = null; + borrowedBy.clear(); return true; } @@ -236,22 +236,13 @@ public class DefaultPooledObject<T> implements PooledObject<T> { @Override public void use() { lastUseTime = System.currentTimeMillis(); - usedBy = new Exception("The last code to use this object was:"); + usedBy.fillInStackTrace(); } @Override public void printStackTrace(final PrintWriter writer) { - boolean written = false; - final Exception borrowedByCopy = this.borrowedBy; - if (borrowedByCopy != null) { - borrowedByCopy.printStackTrace(writer); - written = true; - } - final Exception usedByCopy = this.usedBy; - if (usedByCopy != null) { - usedByCopy.printStackTrace(writer); - written = true; - } + boolean written = borrowedBy.printStackTrace(writer); + written |= usedBy.printStackTrace(writer); if (written) { writer.flush(); } @@ -287,42 +278,4 @@ public class DefaultPooledObject<T> implements PooledObject<T> { this.logAbandoned = logAbandoned; } - /** - * Used to track how an object was obtained from the pool (the stack trace - * of the exception will show which code borrowed the object) and when the - * object was borrowed. - */ - static class AbandonedObjectCreatedException extends Exception { - - private static final long serialVersionUID = 7398692158058772916L; - - /** Date format */ - //@GuardedBy("format") - private static final SimpleDateFormat format = new SimpleDateFormat - ("'Pooled object created' yyyy-MM-dd HH:mm:ss Z " + - "'by the following code has not been returned to the pool:'"); - - private final long _createdTime; - - /** - * Create a new instance. - * <p> - * @see Exception#Exception() - */ - public AbandonedObjectCreatedException() { - super(); - _createdTime = System.currentTimeMillis(); - } - - // Override getMessage to avoid creating objects and formatting - // dates unless the log message will actually be used. - @Override - public String getMessage() { - String msg; - synchronized(format) { - msg = format.format(new Date(_createdTime)); - } - return msg; - } - } } http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/main/java/org/apache/commons/pool2/impl/SecurityManagerCallStack.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/pool2/impl/SecurityManagerCallStack.java b/src/main/java/org/apache/commons/pool2/impl/SecurityManagerCallStack.java new file mode 100644 index 0000000..9d0a8d7 --- /dev/null +++ b/src/main/java/org/apache/commons/pool2/impl/SecurityManagerCallStack.java @@ -0,0 +1,107 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.commons.pool2.impl; + +import java.io.PrintWriter; +import java.lang.ref.WeakReference; +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.text.DateFormat; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.List; + +/** + * CallStack strategy using a {@link SecurityManager}. Obtaining the current call stack is much faster via a + * SecurityManger, but access to the underlying method may be restricted by the current SecurityManager. In environments + * where a SecurityManager cannot be created, {@link ThrowableCallStack} should be used instead. + * + * @see RuntimePermission + * @see SecurityManager#getClassContext() + * @since 2.4.3 + */ +public class SecurityManagerCallStack implements CallStack { + + private final String messageFormat; + //@GuardedBy("dateFormat") + private final DateFormat dateFormat; + private final PrivateSecurityManager securityManager; + + private volatile Snapshot snapshot; + + public SecurityManagerCallStack(final String messageFormat, final boolean useTimestamp) { + this.messageFormat = messageFormat; + this.dateFormat = useTimestamp ? new SimpleDateFormat(messageFormat) : null; + this.securityManager = AccessController.doPrivileged(new PrivilegedAction<PrivateSecurityManager>() { + @Override + public PrivateSecurityManager run() { + return new PrivateSecurityManager(); + } + }); + } + + @Override + public boolean printStackTrace(PrintWriter writer) { + final Snapshot snapshot = this.snapshot; + if (snapshot == null) { + return false; + } + final String message; + if (dateFormat == null) { + message = messageFormat; + } else { + synchronized (dateFormat) { + message = dateFormat.format(snapshot.timestamp); + } + } + writer.println(message); + for (final WeakReference<Class<?>> reference : snapshot.stack) { + writer.println(reference.get()); + } + return true; + } + + @Override + public void fillInStackTrace() { + snapshot = new Snapshot(securityManager.getCallStack()); + } + + @Override + public void clear() { + snapshot = null; + } + + private static class PrivateSecurityManager extends SecurityManager { + private List<WeakReference<Class<?>>> getCallStack() { + final Class[] classes = getClassContext(); + final List<WeakReference<Class<?>>> stack = new ArrayList<WeakReference<Class<?>>>(classes.length); + for (final Class klass : classes) { + stack.add(new WeakReference<Class<?>>(klass)); + } + return stack; + } + } + + private static class Snapshot { + private final long timestamp = System.currentTimeMillis(); + private final List<WeakReference<Class<?>>> stack; + + private Snapshot(List<WeakReference<Class<?>>> stack) { + this.stack = stack; + } + } +} http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java b/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java new file mode 100644 index 0000000..98c733f --- /dev/null +++ b/src/main/java/org/apache/commons/pool2/impl/ThrowableCallStack.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.commons.pool2.impl; + +import java.io.PrintWriter; +import java.text.DateFormat; +import java.text.SimpleDateFormat; + +/** + * CallStack strategy that uses the stack trace from a {@link Throwable}. While being the most portable method of + * obtaining the current call stack, this is also the slowest way to do it. In environments where a new SecurityManager + * can be created, it is preferred to use {@link SecurityManagerCallStack}. + * + * @see Throwable#fillInStackTrace() + * @since 2.4.3 + */ +public class ThrowableCallStack implements CallStack { + + private final String messageFormat; + //@GuardedBy("dateFormat") + private final DateFormat dateFormat; + + private volatile Snapshot snapshot; + + public ThrowableCallStack(final String messageFormat, final boolean useTimestamp) { + this.messageFormat = messageFormat; + this.dateFormat = useTimestamp ? new SimpleDateFormat(messageFormat) : null; + } + + @Override + public synchronized boolean printStackTrace(PrintWriter writer) { + Snapshot snapshot = this.snapshot; + if (snapshot == null) { + return false; + } + final String message; + if (dateFormat == null) { + message = messageFormat; + } else { + synchronized (dateFormat) { + message = dateFormat.format(snapshot.timestamp); + } + } + writer.println(message); + snapshot.printStackTrace(writer); + return true; + } + + @Override + public void fillInStackTrace() { + snapshot = new Snapshot(); + } + + @Override + public void clear() { + snapshot = null; + } + + private static class Snapshot extends Throwable { + private final long timestamp = System.currentTimeMillis(); + } +} http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/test/java/org/apache/commons/pool2/impl/CallStackTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/pool2/impl/CallStackTest.java b/src/test/java/org/apache/commons/pool2/impl/CallStackTest.java new file mode 100644 index 0000000..d3df55d --- /dev/null +++ b/src/test/java/org/apache/commons/pool2/impl/CallStackTest.java @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.commons.pool2.impl; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.io.PrintWriter; +import java.io.StringWriter; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +@RunWith(Parameterized.class) +public class CallStackTest { + + private final CallStack stack; + private final StringWriter writer = new StringWriter(); + + public CallStackTest(CallStack stack) { + this.stack = stack; + } + + @Parameterized.Parameters + public static Object[] data() { + return new Object[]{ + new ThrowableCallStack("Test", false), + new SecurityManagerCallStack("Test", false) + }; + } + + @Test + public void testPrintClearedStackTraceIsNoOp() throws Exception { + stack.fillInStackTrace(); + stack.clear(); + stack.printStackTrace(new PrintWriter(writer)); + String stackTrace = writer.toString(); + assertEquals("", stackTrace); + } + + @Test + public void testPrintFilledStackTrace() throws Exception { + stack.fillInStackTrace(); + stack.printStackTrace(new PrintWriter(writer)); + String stackTrace = writer.toString(); + assertTrue(stackTrace.contains(getClass().getName())); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/commons-pool/blob/3994baf0/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObjectInfo.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObjectInfo.java b/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObjectInfo.java index 584dd82..42f2b17 100644 --- a/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObjectInfo.java +++ b/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObjectInfo.java @@ -16,14 +16,13 @@ */ package org.apache.commons.pool2.impl; -import java.text.SimpleDateFormat; -import java.util.Set; - -import org.apache.commons.pool2.impl.DefaultPooledObject.AbandonedObjectCreatedException; import org.apache.commons.pool2.impl.TestGenericObjectPool.SimpleFactory; import org.junit.Assert; import org.junit.Test; +import java.text.SimpleDateFormat; +import java.util.Set; + public class TestDefaultPooledObjectInfo { @Test @@ -120,17 +119,13 @@ public class TestDefaultPooledObjectInfo { new GenericObjectPoolConfig(), abandonedConfig); - try { - pool.borrowObject(); - //pool.returnObject(s1); // Object not returned, causes abandoned object created exception - } catch (final AbandonedObjectCreatedException e) { - // do nothing. We will print the stack trace later - } + pool.borrowObject(); + //pool.returnObject(s1); // Object not returned, causes abandoned object created exception final Set<DefaultPooledObjectInfo> strings = pool.listAllObjects(); final DefaultPooledObjectInfo s1Info = strings.iterator().next(); final String lastBorrowTrace = s1Info.getLastBorrowTrace(); - Assert.assertTrue(lastBorrowTrace.startsWith(AbandonedObjectCreatedException.class.getName())); + Assert.assertTrue(lastBorrowTrace.startsWith("Pooled object created")); } }
