You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tr...@apache.org on 2006/11/28 03:51:00 UTC
svn commit: r479860 - in
/mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio:
SocketAcceptor.java SocketConnector.java SocketIoProcessor.java
Author: trustin
Date: Mon Nov 27 18:50:59 2006
New Revision: 479860
URL: http://svn.apache.org/viewvc?view=rev&rev=479860
Log:
Related issue: DIRMINA-309 (Decreasing performance in case of fewer number of connections)
* Changed selector fields to final and make it opened when the service is constructed and closed when finialized by VM
Modified:
mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketAcceptor.java
mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketConnector.java
mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketIoProcessor.java
Modified: mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketAcceptor.java
URL: http://svn.apache.org/viewvc/mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketAcceptor.java?view=diff&rev=479860&r1=479859&r2=479860
==============================================================================
--- mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketAcceptor.java (original)
+++ mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketAcceptor.java Mon Nov 27 18:50:59 2006
@@ -71,10 +71,7 @@
private final SocketIoProcessor[] ioProcessors;
private final int processorCount;
- /**
- * @noinspection FieldAccessedSynchronizedAndUnsynchronized
- */
- private Selector selector;
+ private final Selector selector;
private Worker worker;
private int processorDistributor = 0;
@@ -126,6 +123,15 @@
}
}
+ try
+ {
+ this.selector = Selector.open();
+ }
+ catch( IOException e )
+ {
+ throw new RuntimeIOException( "Failed to open a selector.", e );
+ }
+
// Set other properties and initialize
this.executor = executor;
this.processorCount = processorCount;
@@ -137,6 +143,19 @@
}
}
+ protected void finalize() throws Throwable
+ {
+ super.finalize();
+ try
+ {
+ selector.close();
+ }
+ catch( IOException e )
+ {
+ ExceptionMonitor.getInstance().exceptionCaught( e );
+ }
+ }
+
protected Class<? extends SocketAddress> getAddressType()
{
return InetSocketAddress.class;
@@ -197,9 +216,7 @@
RegistrationRequest request = new RegistrationRequest();
registerQueue.offer( request );
-
startupWorker();
-
selector.wakeup();
synchronized( request )
@@ -243,13 +260,12 @@
}
- private synchronized void startupWorker() throws IOException
+ private synchronized void startupWorker()
{
synchronized( lock )
{
if( worker == null )
{
- selector = Selector.open();
worker = new Worker();
executor.execute( new NamePreservingRunnable( worker ) );
@@ -261,20 +277,8 @@
{
CancellationRequest request = new CancellationRequest();
- try
- {
- startupWorker();
- }
- catch( IOException e )
- {
- // IOException is thrown only when Worker thread is not
- // running and failed to open a selector. We simply throw
- // IllegalArgumentException here because we can simply
- // conclude that nothing is bound to the selector.
- throw new IllegalArgumentException( "Address not bound: " + getLocalAddress() );
- }
-
cancelQueue.offer( request );
+ startupWorker();
selector.wakeup();
synchronized( request )
@@ -330,18 +334,6 @@
cancelQueue.isEmpty() )
{
worker = null;
- try
- {
- selector.close();
- }
- catch( IOException e )
- {
- ExceptionMonitor.getInstance().exceptionCaught( e );
- }
- finally
- {
- selector = null;
- }
break;
}
}
Modified: mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketConnector.java
URL: http://svn.apache.org/viewvc/mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketConnector.java?view=diff&rev=479860&r1=479859&r2=479860
==============================================================================
--- mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketConnector.java (original)
+++ mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketConnector.java Mon Nov 27 18:50:59 2006
@@ -36,6 +36,7 @@
import org.apache.mina.common.ExceptionMonitor;
import org.apache.mina.common.IoConnector;
import org.apache.mina.common.IoSessionConfig;
+import org.apache.mina.common.RuntimeIOException;
import org.apache.mina.common.support.AbstractIoFilterChain;
import org.apache.mina.common.support.BaseIoConnector;
import org.apache.mina.common.support.DefaultConnectFuture;
@@ -63,11 +64,8 @@
private final SocketIoProcessor[] ioProcessors;
private final int processorCount;
private final Executor executor;
+ private final Selector selector;
- /**
- * @noinspection FieldAccessedSynchronizedAndUnsynchronized
- */
- private Selector selector;
private Worker worker;
private int processorDistributor = 0;
private int workerTimeout = 60; // 1 min.
@@ -93,6 +91,15 @@
{
throw new IllegalArgumentException( "Must have at least one processor" );
}
+
+ try
+ {
+ this.selector = Selector.open();
+ }
+ catch( IOException e )
+ {
+ throw new RuntimeIOException( "Failed to open a selector.", e );
+ }
this.executor = executor;
this.processorCount = processorCount;
@@ -104,6 +111,19 @@
}
}
+ protected void finalize() throws Throwable
+ {
+ super.finalize();
+ try
+ {
+ selector.close();
+ }
+ catch( IOException e )
+ {
+ ExceptionMonitor.getInstance().exceptionCaught( e );
+ }
+ }
+
protected Class<? extends SocketAddress> getAddressType()
{
return InetSocketAddress.class;
@@ -184,26 +204,7 @@
}
ConnectionRequest request = new ConnectionRequest( ch );
- synchronized( lock )
- {
- try
- {
- startupWorker();
- }
- catch( IOException e )
- {
- try
- {
- ch.close();
- }
- catch( IOException e2 )
- {
- ExceptionMonitor.getInstance().exceptionCaught( e2 );
- }
-
- return DefaultConnectFuture.newFailedFuture( e );
- }
- }
+ startupWorker();
connectQueue.offer( request );
selector.wakeup();
@@ -211,13 +212,15 @@
return request;
}
- private synchronized void startupWorker() throws IOException
+ private void startupWorker()
{
- if( worker == null )
+ synchronized( lock )
{
- selector = Selector.open();
- worker = new Worker();
- executor.execute( new NamePreservingRunnable( worker ) );
+ if( worker == null )
+ {
+ worker = new Worker();
+ executor.execute( new NamePreservingRunnable( worker ) );
+ }
}
}
@@ -381,18 +384,6 @@
connectQueue.isEmpty() )
{
worker = null;
- try
- {
- selector.close();
- }
- catch( IOException e )
- {
- ExceptionMonitor.getInstance().exceptionCaught( e );
- }
- finally
- {
- selector = null;
- }
break;
}
}
Modified: mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketIoProcessor.java
URL: http://svn.apache.org/viewvc/mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketIoProcessor.java?view=diff&rev=479860&r1=479859&r2=479860
==============================================================================
--- mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketIoProcessor.java (original)
+++ mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio/SocketIoProcessor.java Mon Nov 27 18:50:59 2006
@@ -34,6 +34,7 @@
import org.apache.mina.common.IdleStatus;
import org.apache.mina.common.IoService;
import org.apache.mina.common.IoSession;
+import org.apache.mina.common.RuntimeIOException;
import org.apache.mina.common.WriteTimeoutException;
import org.apache.mina.common.IoFilter.WriteRequest;
import org.apache.mina.common.support.IoServiceListenerSupport;
@@ -51,10 +52,7 @@
private final String threadName;
private final Executor executor;
- /**
- * @noinspection FieldAccessedSynchronizedAndUnsynchronized
- */
- private Selector selector;
+ private final Selector selector;
private final Queue<SocketSessionImpl> newSessions = new ConcurrentLinkedQueue<SocketSessionImpl>();
private final Queue<SocketSessionImpl> removingSessions = new ConcurrentLinkedQueue<SocketSessionImpl>();
@@ -68,33 +66,53 @@
{
this.threadName = threadName;
this.executor = executor;
+ try
+ {
+ this.selector = Selector.open();
+ }
+ catch( IOException e )
+ {
+ throw new RuntimeIOException( "Failed to open a selector.", e );
+ }
+ }
+
+ protected void finalize() throws Throwable
+ {
+ super.finalize();
+ try
+ {
+ selector.close();
+ }
+ catch( IOException e )
+ {
+ ExceptionMonitor.getInstance().exceptionCaught( e );
+ }
}
- void addNew( SocketSessionImpl session ) throws IOException
+ void addNew( SocketSessionImpl session )
{
newSessions.offer( session );
startupWorker();
}
- void remove( SocketSessionImpl session ) throws IOException
+ void remove( SocketSessionImpl session )
{
scheduleRemove( session );
startupWorker();
}
- private void startupWorker() throws IOException
+ private void startupWorker()
{
synchronized( lock )
{
if( worker == null )
{
- selector = Selector.open();
worker = new Worker();
executor.execute( new NamePreservingRunnable( worker ) );
}
- selector.wakeup();
}
+ selector.wakeup();
}
void flush( SocketSessionImpl session )
@@ -549,20 +567,6 @@
if( selector.keys().isEmpty() && newSessions.isEmpty() )
{
worker = null;
-
- try
- {
- selector.close();
- }
- catch( IOException e )
- {
- ExceptionMonitor.getInstance().exceptionCaught( e );
- }
- finally
- {
- selector = null;
- }
-
break;
}
}
@@ -584,5 +588,4 @@
}
}
}
-
}
Re: svn commit: r479860 - in /mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio: SocketAcceptor.java SocketConnector.java SocketIoProcessor.java
Posted by Trustin Lee <tr...@gmail.com>.
On 11/29/06, peter royal <pr...@apache.org> wrote:
>
> On Nov 27, 2006, at 6:51 PM, trustin@apache.org wrote:
> > + protected void finalize() throws Throwable
> > + {
> > + super.finalize();
> > + try
> > + {
> > + selector.close();
> > + }
> > + catch( IOException e )
> > + {
> > + ExceptionMonitor.getInstance().exceptionCaught( e );
> > + }
> > + }
>
> How about an explicit .close() or similarly-named method so its not
> necessary to rely on finalization?
>
> I'm against having finalization be the only way to have the selector
> be closed, as it is bad practice to use finalizers as destructors.
I thought the side effect of finilizers is very small considering the number
of SocketIoProcessors and other worker threads that uses a Selector. They
will usually consume less than 10 file descriptors, so it shouldn't be a
problem for most applications. Providing an additional shutdown method
would be a great idea, too, and I was going to provide one. That's why I
checked in this code to only trunk. We can code a little bit more and
provide such a method to resolve DIRMINA-316 (
http://issues.apache.org/jira/browse/DIRMINA-316).
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP key fingerprints:
* E167 E6AF E73A CBCE EE41 4A29 544D DE48 FE95 4E7E
* B693 628E 6047 4F8F CFA4 455E 1C62 A7DC 0255 ECA6
Re: svn commit: r479860 - in /mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio: SocketAcceptor.java SocketConnector.java SocketIoProcessor.java
Posted by Maarten Bosteels <mb...@gmail.com>.
I agree.
Maarten
On 11/28/06, peter royal <pr...@apache.org> wrote:
>
> On Nov 27, 2006, at 6:51 PM, trustin@apache.org wrote:
> > + protected void finalize() throws Throwable
> > + {
> > + super.finalize();
> > + try
> > + {
> > + selector.close();
> > + }
> > + catch( IOException e )
> > + {
> > + ExceptionMonitor.getInstance().exceptionCaught( e );
> > + }
> > + }
>
> How about an explicit .close() or similarly-named method so its not
> necessary to rely on finalization?
>
> I'm against having finalization be the only way to have the selector
> be closed, as it is bad practice to use finalizers as destructors.
>
> -pete
>
>
> --
> proyal@apache.org - http://fotap.org/~osi
>
>
>
>
>
>
Re: svn commit: r479860 - in /mina/trunk/core/src/main/java/org/apache/mina/transport/socket/nio: SocketAcceptor.java SocketConnector.java SocketIoProcessor.java
Posted by peter royal <pr...@apache.org>.
On Nov 27, 2006, at 6:51 PM, trustin@apache.org wrote:
> + protected void finalize() throws Throwable
> + {
> + super.finalize();
> + try
> + {
> + selector.close();
> + }
> + catch( IOException e )
> + {
> + ExceptionMonitor.getInstance().exceptionCaught( e );
> + }
> + }
How about an explicit .close() or similarly-named method so its not
necessary to rely on finalization?
I'm against having finalization be the only way to have the selector
be closed, as it is bad practice to use finalizers as destructors.
-pete
--
proyal@apache.org - http://fotap.org/~osi