* Misuse warnings are logged at WARN level now (with higher threshold)

* Extracted duplicate code into SharedResourceMisuseDetector
This commit is contained in:
Trustin Lee 2010-04-06 09:23:25 +00:00
parent 11e5e2ba56
commit 8474026c92
3 changed files with 73 additions and 31 deletions

View File

@ -25,8 +25,6 @@ import java.util.concurrent.Semaphore;
import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicLong;
import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.Channel;
@ -41,6 +39,7 @@ import org.jboss.netty.util.DefaultObjectSizeEstimator;
import org.jboss.netty.util.ObjectSizeEstimator; import org.jboss.netty.util.ObjectSizeEstimator;
import org.jboss.netty.util.internal.ConcurrentIdentityHashMap; import org.jboss.netty.util.internal.ConcurrentIdentityHashMap;
import org.jboss.netty.util.internal.LinkedTransferQueue; import org.jboss.netty.util.internal.LinkedTransferQueue;
import org.jboss.netty.util.internal.SharedResourceMisuseDetector;
/** /**
* A {@link ThreadPoolExecutor} which blocks the task submission when there's * A {@link ThreadPoolExecutor} which blocks the task submission when there's
@ -86,10 +85,8 @@ public class MemoryAwareThreadPoolExecutor extends ThreadPoolExecutor {
private static final InternalLogger logger = private static final InternalLogger logger =
InternalLoggerFactory.getInstance(MemoryAwareThreadPoolExecutor.class); InternalLoggerFactory.getInstance(MemoryAwareThreadPoolExecutor.class);
// I'd say 64 active event thread pools are obvious misuse. private static final SharedResourceMisuseDetector misuseDetector =
private static final int MISUSE_WARNING_THRESHOLD = 64; new SharedResourceMisuseDetector(MemoryAwareThreadPoolExecutor.class);
private static final AtomicInteger activeInstances = new AtomicInteger();
private static final AtomicBoolean loggedMisuseWarning = new AtomicBoolean();
private volatile Settings settings; private volatile Settings settings;
@ -200,22 +197,13 @@ public class MemoryAwareThreadPoolExecutor extends ThreadPoolExecutor {
objectSizeEstimator, maxChannelMemorySize, maxTotalMemorySize); objectSizeEstimator, maxChannelMemorySize, maxTotalMemorySize);
// Misuse check // Misuse check
int activeInstances = MemoryAwareThreadPoolExecutor.activeInstances.incrementAndGet(); misuseDetector.increase();
if (activeInstances >= MISUSE_WARNING_THRESHOLD &&
loggedMisuseWarning.compareAndSet(false, true)) {
logger.debug(
"There are too many active " +
MemoryAwareThreadPoolExecutor.class.getSimpleName() +
" instances (" + activeInstances + ") - you should share " +
"the small number of instances to avoid excessive resource " +
"consumption.");
}
} }
@Override @Override
protected void terminated() { protected void terminated() {
super.terminated(); super.terminated();
activeInstances.decrementAndGet(); misuseDetector.decrease();
} }
/** /**

View File

@ -32,6 +32,7 @@ import org.jboss.netty.logging.InternalLogger;
import org.jboss.netty.logging.InternalLoggerFactory; import org.jboss.netty.logging.InternalLoggerFactory;
import org.jboss.netty.util.internal.ConcurrentIdentityHashMap; import org.jboss.netty.util.internal.ConcurrentIdentityHashMap;
import org.jboss.netty.util.internal.ReusableIterator; import org.jboss.netty.util.internal.ReusableIterator;
import org.jboss.netty.util.internal.SharedResourceMisuseDetector;
/** /**
* A {@link Timer} optimized for approximated I/O timeout scheduling. * A {@link Timer} optimized for approximated I/O timeout scheduling.
@ -77,10 +78,8 @@ public class HashedWheelTimer implements Timer {
InternalLoggerFactory.getInstance(HashedWheelTimer.class); InternalLoggerFactory.getInstance(HashedWheelTimer.class);
private static final AtomicInteger id = new AtomicInteger(); private static final AtomicInteger id = new AtomicInteger();
// I'd say 64 active timer threads are obvious misuse. private static final SharedResourceMisuseDetector misuseDetector =
private static final int MISUSE_WARNING_THRESHOLD = 64; new SharedResourceMisuseDetector(HashedWheelTimer.class);
private static final AtomicInteger activeInstances = new AtomicInteger();
private static final AtomicBoolean loggedMisuseWarning = new AtomicBoolean();
private final Worker worker = new Worker(); private final Worker worker = new Worker();
final Thread workerThread; final Thread workerThread;
@ -204,15 +203,7 @@ public class HashedWheelTimer implements Timer {
worker, "Hashed wheel timer #" + id.incrementAndGet())); worker, "Hashed wheel timer #" + id.incrementAndGet()));
// Misuse check // Misuse check
int activeInstances = HashedWheelTimer.activeInstances.incrementAndGet(); misuseDetector.increase();
if (activeInstances >= MISUSE_WARNING_THRESHOLD &&
loggedMisuseWarning.compareAndSet(false, true)) {
logger.debug(
"There are too many active " +
HashedWheelTimer.class.getSimpleName() + " instances (" +
activeInstances + ") - you should share the small number " +
"of instances to avoid excessive resource consumption.");
}
} }
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@ -288,7 +279,7 @@ public class HashedWheelTimer implements Timer {
Thread.currentThread().interrupt(); Thread.currentThread().interrupt();
} }
activeInstances.decrementAndGet(); misuseDetector.decrease();
Set<Timeout> unprocessedTimeouts = new HashSet<Timeout>(); Set<Timeout> unprocessedTimeouts = new HashSet<Timeout>();
for (Set<HashedWheelTimeout> bucket: wheel) { for (Set<HashedWheelTimeout> bucket: wheel) {

View File

@ -0,0 +1,63 @@
/*
* Copyright 2009 Red Hat, Inc.
*
* Red Hat 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.jboss.netty.util.internal;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import org.jboss.netty.logging.InternalLogger;
import org.jboss.netty.logging.InternalLoggerFactory;
/**
* Warn when user creates too many instances to avoid {@link OutOfMemoryError}.
*
* @author <a href="http://www.jboss.org/netty/">The Netty Project</a>
* @author <a href="http://gleamynode.net/">Trustin Lee</a>
* @version $Rev$, $Date$
*/
public class SharedResourceMisuseDetector {
private static final int MAX_ACTIVE_INSTANCES = 256;
private static final InternalLogger logger =
InternalLoggerFactory.getInstance(SharedResourceMisuseDetector.class);
private final Class<?> type;
private final AtomicLong activeInstances = new AtomicLong();
private final AtomicBoolean logged = new AtomicBoolean();
public SharedResourceMisuseDetector(Class<?> type) {
if (type == null) {
throw new NullPointerException("type");
}
this.type = type;
}
public void increase() {
if (activeInstances.incrementAndGet() > MAX_ACTIVE_INSTANCES) {
if (logged.compareAndSet(false, true)) {
logger.warn(
"You are creating too many " + type.getSimpleName() +
" instances. " + type.getSimpleName() +
" is a shared resource that must be reused across the" +
" application, so that only a few instances are created.");
}
}
}
public void decrease() {
activeInstances.decrementAndGet();
}
}