[jitsi-dev] Decreasing number of threads in ice4j


#1

Hi,
While testing ice4j and PseudoTCP I've noticed that each connection leads to quite a few new threads being created. When I was running some tests with many concurrent connections it started throwing errors because it couldn't create more threads. Since each thread introduces a non-negligible context switching cost and memory cost I figured I'd give it a shot at reducing the number of threads created when multiple connections are created. For instance I think that StunKeepAliveThread is a good candidate for code that should benefit from being run by a ScheduledExecutorService instead (since it spends most of its time sleeping anyway).

Is this a refactoring that you'd be interested in? If so, I'd gladly give it a shot. Since this probably will lead to quite a few changes across many files it's probably easier for all of us if we either agree on some patch format that I should send the changes in or that you give me commit access to the SVN repository.

Regards
Carl


#2

Hi,

Hi,

While testing ice4j and PseudoTCP I've noticed that each connection leads to
quite a few new threads being created. When I was running some tests with
many concurrent connections it started throwing errors because it couldn't
create more threads. Since each thread introduces a non-negligible context
switching cost and memory cost I figured I'd give it a shot at reducing the
number of threads created when multiple connections are created. For
instance I think that StunKeepAliveThread is a good candidate for code that
should benefit from being run by a ScheduledExecutorService instead (since
it spends most of its time sleeping anyway).

Is this a refactoring that you'd be interested in? If so, I'd gladly give it
a shot. Since this probably will lead to quite a few changes across many
files it's probably easier for all of us if we either agree on some patch
format that I should send the changes in or that you give me commit access
to the SVN repository.

I'm not the one to judge about core ice4j threads, but we can discuss
reduction in PseudoTCP if you have any ideas.

Here's how it works now. For each of PseudoTcp streams there are 2
threads being created.
The first one calls recv() method of DatagramSocket in a loop to read
incoming packets from the network.
The second one keeps sleeping in intervals returned by PseudoTcpBase
class and calls method notifyClock() of PseudoTcpBase class. This is
required to handle timeouts and retransmits as the time goes on.

If it would be possible to receive datagrams with timeout, number of
threads may be then reduced from 2 to 1. Is it possible do to such
thing ?

Another question is do you really need to use PseudoTcp ? It was put
in ice4j as an optional feature and you can still use system TCP
sockets with ice4j.

···

2012/8/16 Carl Hasselskog <carl@degoo.com>:

--
Regards, Paweł Domas
.


#3

Hi,
DatagramSocket supports setting a timeout (http://docs.oracle.com/javase/1.4.2/docs/api/java/net/DatagramSocket.html#setSoTimeout(int)), would that help you? In my tests Connector seems to be the biggest source of new threads. A timeout on receiving datagrams would in Connector as well. Then one could rewrite it to use a thread-pool where the task is rescheduled upon timeout.

We want to use PseudoTCP because from what I understand the success rate of whole punching is much greater using UDP than TCP.

Regards
Carl

···

-----Original Message-----

From: Paweł Domas [mailto:paweldomas@gmail.com]

Sent: den 19 augusti 2012 19:23
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,

2012/8/16 Carl Hasselskog <carl@degoo.com>:

Hi,

While testing ice4j and PseudoTCP I've noticed that each connection
leads to quite a few new threads being created. When I was running
some tests with many concurrent connections it started throwing errors
because it couldn't create more threads. Since each thread introduces
a non-negligible context switching cost and memory cost I figured I'd
give it a shot at reducing the number of threads created when multiple
connections are created. For instance I think that StunKeepAliveThread
is a good candidate for code that should benefit from being run by a
ScheduledExecutorService instead (since it spends most of its time sleeping anyway).

Is this a refactoring that you'd be interested in? If so, I'd gladly
give it a shot. Since this probably will lead to quite a few changes
across many files it's probably easier for all of us if we either
agree on some patch format that I should send the changes in or that
you give me commit access to the SVN repository.

I'm not the one to judge about core ice4j threads, but we can discuss reduction in PseudoTCP if you have any ideas.

Here's how it works now. For each of PseudoTcp streams there are 2 threads being created.
The first one calls recv() method of DatagramSocket in a loop to read incoming packets from the network.
The second one keeps sleeping in intervals returned by PseudoTcpBase class and calls method notifyClock() of PseudoTcpBase class. This is required to handle timeouts and retransmits as the time goes on.

If it would be possible to receive datagrams with timeout, number of threads may be then reduced from 2 to 1. Is it possible do to such thing ?

Another question is do you really need to use PseudoTcp ? It was put in ice4j as an optional feature and you can still use system TCP sockets with ice4j.

--
Regards, Paweł Domas

.


#4

I should also add that by doing this change it would also be quite easy to add support for timeouts in read(). At the moment when I read from the InputStream created by the SSLSocket (that I wrap around PseudoTcpJavaSocket) it will wait indefinitely, even if a timeout has been set on the SSLSocket.

Regards
Carl

···

-----Original Message-----

From: Carl Hasselskog [mailto:carl@degoo.com]

Sent: den 19 augusti 2012 22:13
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,
DatagramSocket supports setting a timeout (http://docs.oracle.com/javase/1.4.2/docs/api/java/net/DatagramSocket.html#setSoTimeout(int)), would that help you? In my tests Connector seems to be the biggest source of new threads. A timeout on receiving datagrams would in Connector as well. Then one could rewrite it to use a thread-pool where the task is rescheduled upon timeout.

We want to use PseudoTCP because from what I understand the success rate of whole punching is much greater using UDP than TCP.

Regards
Carl

-----Original Message-----

From: Paweł Domas [mailto:paweldomas@gmail.com]

Sent: den 19 augusti 2012 19:23
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,

2012/8/16 Carl Hasselskog <carl@degoo.com>:

Hi,

While testing ice4j and PseudoTCP I've noticed that each connection
leads to quite a few new threads being created. When I was running
some tests with many concurrent connections it started throwing errors
because it couldn't create more threads. Since each thread introduces
a non-negligible context switching cost and memory cost I figured I'd
give it a shot at reducing the number of threads created when multiple
connections are created. For instance I think that StunKeepAliveThread
is a good candidate for code that should benefit from being run by a
ScheduledExecutorService instead (since it spends most of its time sleeping anyway).

Is this a refactoring that you'd be interested in? If so, I'd gladly
give it a shot. Since this probably will lead to quite a few changes
across many files it's probably easier for all of us if we either
agree on some patch format that I should send the changes in or that
you give me commit access to the SVN repository.

I'm not the one to judge about core ice4j threads, but we can discuss reduction in PseudoTCP if you have any ideas.

Here's how it works now. For each of PseudoTcp streams there are 2 threads being created.
The first one calls recv() method of DatagramSocket in a loop to read incoming packets from the network.
The second one keeps sleeping in intervals returned by PseudoTcpBase class and calls method notifyClock() of PseudoTcpBase class. This is required to handle timeouts and retransmits as the time goes on.

If it would be possible to receive datagrams with timeout, number of threads may be then reduced from 2 to 1. Is it possible do to such thing ?

Another question is do you really need to use PseudoTcp ? It was put in ice4j as an optional feature and you can still use system TCP sockets with ice4j.

--
Regards, Paweł Domas

.


#5

To be more specific: it either waits at read_notify.wait(); in read(byte[] buffer, int offset, int length) (when reading) or at ackNotify.wait() in flush() (when writing). I think we need some timeout here, in order to detect broken connections.

Regards
Carl

···

-----Original Message-----

From: Carl Hasselskog [mailto:carl@degoo.com]

Sent: den 20 augusti 2012 15:32
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

I should also add that by doing this change it would also be quite easy to add support for timeouts in read(). At the moment when I read from the InputStream created by the SSLSocket (that I wrap around PseudoTcpJavaSocket) it will wait indefinitely, even if a timeout has been set on the SSLSocket.

Regards
Carl

-----Original Message-----

From: Carl Hasselskog [mailto:carl@degoo.com]

Sent: den 19 augusti 2012 22:13
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,
DatagramSocket supports setting a timeout (http://docs.oracle.com/javase/1.4.2/docs/api/java/net/DatagramSocket.html#setSoTimeout(int)), would that help you? In my tests Connector seems to be the biggest source of new threads. A timeout on receiving datagrams would in Connector as well. Then one could rewrite it to use a thread-pool where the task is rescheduled upon timeout.

We want to use PseudoTCP because from what I understand the success rate of whole punching is much greater using UDP than TCP.

Regards
Carl

-----Original Message-----

From: Paweł Domas [mailto:paweldomas@gmail.com]

Sent: den 19 augusti 2012 19:23
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,

2012/8/16 Carl Hasselskog <carl@degoo.com>:

Hi,

While testing ice4j and PseudoTCP I've noticed that each connection
leads to quite a few new threads being created. When I was running
some tests with many concurrent connections it started throwing errors
because it couldn't create more threads. Since each thread introduces
a non-negligible context switching cost and memory cost I figured I'd
give it a shot at reducing the number of threads created when multiple
connections are created. For instance I think that StunKeepAliveThread
is a good candidate for code that should benefit from being run by a
ScheduledExecutorService instead (since it spends most of its time sleeping anyway).

Is this a refactoring that you'd be interested in? If so, I'd gladly
give it a shot. Since this probably will lead to quite a few changes
across many files it's probably easier for all of us if we either
agree on some patch format that I should send the changes in or that
you give me commit access to the SVN repository.

I'm not the one to judge about core ice4j threads, but we can discuss reduction in PseudoTCP if you have any ideas.

Here's how it works now. For each of PseudoTcp streams there are 2 threads being created.
The first one calls recv() method of DatagramSocket in a loop to read incoming packets from the network.
The second one keeps sleeping in intervals returned by PseudoTcpBase class and calls method notifyClock() of PseudoTcpBase class. This is required to handle timeouts and retransmits as the time goes on.

If it would be possible to receive datagrams with timeout, number of threads may be then reduced from 2 to 1. Is it possible do to such thing ?

Another question is do you really need to use PseudoTcp ? It was put in ice4j as an optional feature and you can still use system TCP sockets with ice4j.

--
Regards, Paweł Domas

.


#6

Hi,

To be more specific: it either waits at read_notify.wait(); in read(byte[] buffer, int offset, int length) (when reading) or at ackNotify.wait() in flush() (when writing). I think we need some timeout here, in order to detect broken connections.

Regards
Carl

From: Carl Hasselskog [mailto:carl@degoo.com]
Sent: den 20 augusti 2012 15:32
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

I should also add that by doing this change it would also be quite easy to add support for timeouts in read(). At the moment when I read from the InputStream created by the SSLSocket (that I wrap around PseudoTcpJavaSocket) it will wait indefinitely, even if a timeout has been set on the SSLSocket.

Regards
Carl

From: Carl Hasselskog [mailto:carl@degoo.com]
Sent: den 19 augusti 2012 22:13
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,
DatagramSocket supports setting a timeout (http://docs.oracle.com/javase/1.4.2/docs/api/java/net/DatagramSocket.html#setSoTimeout(int)), would that help you? In my tests Connector seems to be the biggest source of new threads. A timeout on receiving datagrams would in Connector as well. Then one could rewrite it to use a thread-pool where the task is rescheduled upon timeout.

We want to use PseudoTCP because from what I understand the success rate of whole punching is much greater using UDP than TCP.

Regards
Carl

From: Paweł Domas [mailto:paweldomas@gmail.com]
Sent: den 19 augusti 2012 19:23
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,

Hi,

While testing ice4j and PseudoTCP I've noticed that each connection
leads to quite a few new threads being created. When I was running
some tests with many concurrent connections it started throwing errors
because it couldn't create more threads. Since each thread introduces
a non-negligible context switching cost and memory cost I figured I'd
give it a shot at reducing the number of threads created when multiple
connections are created. For instance I think that StunKeepAliveThread
is a good candidate for code that should benefit from being run by a
ScheduledExecutorService instead (since it spends most of its time sleeping anyway).

Is this a refactoring that you'd be interested in? If so, I'd gladly
give it a shot. Since this probably will lead to quite a few changes
across many files it's probably easier for all of us if we either
agree on some patch format that I should send the changes in or that
you give me commit access to the SVN repository.

I'm not the one to judge about core ice4j threads, but we can discuss reduction in PseudoTCP if you have any ideas.

Here's how it works now. For each of PseudoTcp streams there are 2 threads being created.
The first one calls recv() method of DatagramSocket in a loop to read incoming packets from the network.
The second one keeps sleeping in intervals returned by PseudoTcpBase class and calls method notifyClock() of PseudoTcpBase class. This is required to handle timeouts and retransmits as the time goes on.

If it would be possible to receive datagrams with timeout, number of threads may be then reduced from 2 to 1. Is it possible do to such thing ?

Another question is do you really need to use PseudoTcp ? It was put in ice4j as an optional feature and you can still use system TCP sockets with ice4j.

--
Regards, Paweł Domas

But is it another subject to discuss - "detection of broken
connections" ? or do you mean that this attempt with reducing thread
by setting SoTimeout have some impact on that ?

I had an attempt to reduce threads count with SoTimeout and it
partially worked, but there are problems with SocketTimeoutExceptions
in ice4j sockets that take it as critical exception. When I changed it
to ignore them there were still some problems with it, as I probably
don't understand how sockets in ice4j work.

···

2012/8/20 Carl Hasselskog <carl@degoo.com>:

-----Original Message-----
-----Original Message-----
-----Original Message-----
2012/8/16 Carl Hasselskog <carl@degoo.com>:

--
Regards, Paweł Domas
.


#7

Yes, it's a different discussion but they are somewhat related. By adding support for timeouts we make it a lot easier to support thread-pools rather than having one thread per call.

Regards
Carl

···

-----Original Message-----

From: Paweł Domas [mailto:paweldomas@gmail.com]

Sent: den 20 augusti 2012 15:57
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,

2012/8/20 Carl Hasselskog <carl@degoo.com>:

To be more specific: it either waits at read_notify.wait(); in read(byte[] buffer, int offset, int length) (when reading) or at ackNotify.wait() in flush() (when writing). I think we need some timeout here, in order to detect broken connections.

Regards
Carl

-----Original Message-----
From: Carl Hasselskog [mailto:carl@degoo.com]
Sent: den 20 augusti 2012 15:32
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

I should also add that by doing this change it would also be quite easy to add support for timeouts in read(). At the moment when I read from the InputStream created by the SSLSocket (that I wrap around PseudoTcpJavaSocket) it will wait indefinitely, even if a timeout has been set on the SSLSocket.

Regards
Carl

-----Original Message-----
From: Carl Hasselskog [mailto:carl@degoo.com]
Sent: den 19 augusti 2012 22:13
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,
DatagramSocket supports setting a timeout (http://docs.oracle.com/javase/1.4.2/docs/api/java/net/DatagramSocket.html#setSoTimeout(int)), would that help you? In my tests Connector seems to be the biggest source of new threads. A timeout on receiving datagrams would in Connector as well. Then one could rewrite it to use a thread-pool where the task is rescheduled upon timeout.

We want to use PseudoTCP because from what I understand the success rate of whole punching is much greater using UDP than TCP.

Regards
Carl

-----Original Message-----
From: Paweł Domas [mailto:paweldomas@gmail.com]
Sent: den 19 augusti 2012 19:23
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: Decreasing number of threads in ice4j

Hi,

2012/8/16 Carl Hasselskog <carl@degoo.com>:

Hi,

While testing ice4j and PseudoTCP I've noticed that each connection
leads to quite a few new threads being created. When I was running
some tests with many concurrent connections it started throwing
errors because it couldn't create more threads. Since each thread
introduces a non-negligible context switching cost and memory cost I
figured I'd give it a shot at reducing the number of threads created
when multiple connections are created. For instance I think that
StunKeepAliveThread is a good candidate for code that should benefit
from being run by a ScheduledExecutorService instead (since it spends most of its time sleeping anyway).

Is this a refactoring that you'd be interested in? If so, I'd gladly
give it a shot. Since this probably will lead to quite a few changes
across many files it's probably easier for all of us if we either
agree on some patch format that I should send the changes in or that
you give me commit access to the SVN repository.

I'm not the one to judge about core ice4j threads, but we can discuss reduction in PseudoTCP if you have any ideas.

Here's how it works now. For each of PseudoTcp streams there are 2 threads being created.
The first one calls recv() method of DatagramSocket in a loop to read incoming packets from the network.
The second one keeps sleeping in intervals returned by PseudoTcpBase class and calls method notifyClock() of PseudoTcpBase class. This is required to handle timeouts and retransmits as the time goes on.

If it would be possible to receive datagrams with timeout, number of threads may be then reduced from 2 to 1. Is it possible do to such thing ?

Another question is do you really need to use PseudoTcp ? It was put in ice4j as an optional feature and you can still use system TCP sockets with ice4j.

--
Regards, Paweł Domas

But is it another subject to discuss - "detection of broken connections" ? or do you mean that this attempt with reducing thread by setting SoTimeout have some impact on that ?

I had an attempt to reduce threads count with SoTimeout and it partially worked, but there are problems with SocketTimeoutExceptions in ice4j sockets that take it as critical exception. When I changed it to ignore them there were still some problems with it, as I probably don't understand how sockets in ice4j work.

--
Regards, Paweł Domas