From 8474026c922093d6b2bdc92d21366176c18dcbb9 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Tue, 6 Apr 2010 09:23:25 +0000 Subject: [PATCH] * Misuse warnings are logged at WARN level now (with higher threshold) * Extracted duplicate code into SharedResourceMisuseDetector --- .../MemoryAwareThreadPoolExecutor.java | 22 ++----- .../jboss/netty/util/HashedWheelTimer.java | 19 ++---- .../SharedResourceMisuseDetector.java | 63 +++++++++++++++++++ 3 files changed, 73 insertions(+), 31 deletions(-) create mode 100644 src/main/java/org/jboss/netty/util/internal/SharedResourceMisuseDetector.java diff --git a/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java b/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java index b0206ae546..7fcda017ea 100644 --- a/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java +++ b/src/main/java/org/jboss/netty/handler/execution/MemoryAwareThreadPoolExecutor.java @@ -25,8 +25,6 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; 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.internal.ConcurrentIdentityHashMap; 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 @@ -86,10 +85,8 @@ public class MemoryAwareThreadPoolExecutor extends ThreadPoolExecutor { private static final InternalLogger logger = InternalLoggerFactory.getInstance(MemoryAwareThreadPoolExecutor.class); - // I'd say 64 active event thread pools are obvious misuse. - private static final int MISUSE_WARNING_THRESHOLD = 64; - private static final AtomicInteger activeInstances = new AtomicInteger(); - private static final AtomicBoolean loggedMisuseWarning = new AtomicBoolean(); + private static final SharedResourceMisuseDetector misuseDetector = + new SharedResourceMisuseDetector(MemoryAwareThreadPoolExecutor.class); private volatile Settings settings; @@ -200,22 +197,13 @@ public class MemoryAwareThreadPoolExecutor extends ThreadPoolExecutor { objectSizeEstimator, maxChannelMemorySize, maxTotalMemorySize); // Misuse check - int activeInstances = MemoryAwareThreadPoolExecutor.activeInstances.incrementAndGet(); - 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."); - } + misuseDetector.increase(); } @Override protected void terminated() { super.terminated(); - activeInstances.decrementAndGet(); + misuseDetector.decrease(); } /** diff --git a/src/main/java/org/jboss/netty/util/HashedWheelTimer.java b/src/main/java/org/jboss/netty/util/HashedWheelTimer.java index d2690a0732..c5e9156ec3 100644 --- a/src/main/java/org/jboss/netty/util/HashedWheelTimer.java +++ b/src/main/java/org/jboss/netty/util/HashedWheelTimer.java @@ -32,6 +32,7 @@ import org.jboss.netty.logging.InternalLogger; import org.jboss.netty.logging.InternalLoggerFactory; import org.jboss.netty.util.internal.ConcurrentIdentityHashMap; import org.jboss.netty.util.internal.ReusableIterator; +import org.jboss.netty.util.internal.SharedResourceMisuseDetector; /** * A {@link Timer} optimized for approximated I/O timeout scheduling. @@ -77,10 +78,8 @@ public class HashedWheelTimer implements Timer { InternalLoggerFactory.getInstance(HashedWheelTimer.class); private static final AtomicInteger id = new AtomicInteger(); - // I'd say 64 active timer threads are obvious misuse. - private static final int MISUSE_WARNING_THRESHOLD = 64; - private static final AtomicInteger activeInstances = new AtomicInteger(); - private static final AtomicBoolean loggedMisuseWarning = new AtomicBoolean(); + private static final SharedResourceMisuseDetector misuseDetector = + new SharedResourceMisuseDetector(HashedWheelTimer.class); private final Worker worker = new Worker(); final Thread workerThread; @@ -204,15 +203,7 @@ public class HashedWheelTimer implements Timer { worker, "Hashed wheel timer #" + id.incrementAndGet())); // Misuse check - int activeInstances = HashedWheelTimer.activeInstances.incrementAndGet(); - 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."); - } + misuseDetector.increase(); } @SuppressWarnings("unchecked") @@ -288,7 +279,7 @@ public class HashedWheelTimer implements Timer { Thread.currentThread().interrupt(); } - activeInstances.decrementAndGet(); + misuseDetector.decrease(); Set unprocessedTimeouts = new HashSet(); for (Set bucket: wheel) { diff --git a/src/main/java/org/jboss/netty/util/internal/SharedResourceMisuseDetector.java b/src/main/java/org/jboss/netty/util/internal/SharedResourceMisuseDetector.java new file mode 100644 index 0000000000..f42d21b5a6 --- /dev/null +++ b/src/main/java/org/jboss/netty/util/internal/SharedResourceMisuseDetector.java @@ -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 The Netty Project + * @author Trustin Lee + * @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(); + } +}