Hi Alexey,
I think that we don't reregister single Channel 2 times in AbstractConnectorHandler and then in SelectorHandlerRunner because we uses auxController(ReadController) and ReadController's SelectorHandler in AbstractConnectorHandler#connect(SocketAddress, SocketAddress, K).
Then, in SelectorHandlerRunner, controller.getReadThreadsCount() is equal to 0. So the following changes are perhaps meaningless.
Right? Am I understanding your words correctly?
Index: com/sun/grizzly/SelectorHandlerRunner.java
===================================================================
--- com/sun/grizzly/SelectorHandlerRunner.java (revision 3404)
+++ com/sun/grizzly/SelectorHandlerRunner.java (working copy)
@@ -296,10 +296,19 @@
return true;
}
if ((readyOps & SelectionKey.OP_CONNECT) != 0) {
- if (isLogLevelFine) {
- dolog("OP_CONNECT on ", key);
+ if (controller.getReadThreadsCount() > 0 &&
+ controller.multiReadThreadSelectorHandler.supportsClient(selectorHandler)) {
+ // this case doesn't happen if ConnectorHandler supports connect()'s round robin
+ if (isLogLevelFine) {
+ dolog("OP_CONNECT passed to multi readthread handler on ", key);
+ }
+ controller.multiReadThreadSelectorHandler.onConnectInterest(key, serverCtx);
+ } else {
+ if (isLogLevelFine) {
+ dolog("OP_CONNECT on ", key);
+ }
+ selectorHandler.onConnectInterest(key, serverCtx);
}
- selectorHandler.onConnectInterest(key, serverCtx);
return true;
}
Maybe above changes are redundant because we already support connect()'s round robin in my patch.
Is it better that above code should be removed?
What do you think?
--
Bongjae Chang
----- Original Message -----
From: "Oleksiy Stashok" <Oleksiy.Stashok_at_Sun.COM>
To: <dev_at_grizzly.dev.java.net>
Sent: Wednesday, July 01, 2009 8:51 PM
Subject: Re: Supporing OP_READ round robin on UDP
> Hi Bongjae,
>
> from diff you sent, don't we duplicate the Channel spreading logic in
> AbstractConnectorHandler.connect(SocketAddress, SocketAddress, K) and
> SelectorHandlerRunner?
> I just have filling that we reregister single Channel 2 times in
> AbstractConnectorHandler and then in SelectorHandlerRunner, but I
> could be wrong.
>
> Thanks.
>
> WBR,
> Alexey.
>
>
> On Jul 1, 2009, at 12:31 , Bongjae Chang wrote:
>
>> Hi,
>>
>> I modified the patch again.
>>
>> Current patch support client-side's round-robin on TCP|SSL as well
>> as UDP.
>>
>> In addition, I moved common connect() methods of TCP|UDP|
>> SSLConnectorHandler into AbstractConnectorHandler.
>>
>> And I modified roundRobinCounter to be atomic.
>>
>> In UDP, I could see that the perf was improved clearly. But in TCP,
>> I am not sure.
>>
>> When I tried to disable both server and client side's round-robin in
>> TCP, the perf was best.
>>
>> Maybe I think that the result could be different according to test
>> app or system environments.
>>
>> Anyway, I attached it and simple app for local test.
>>
>> Now, RoundRobinSelectorHandler#supportsProtocol() means server side
>> round-robin's use flag and
>> RoundRobinSelectorHandler#supportsClient() means client size round-
>> robin's use flag.
>>
>> Please review this and advice me again.
>>
>> Thanks!
>>
>> --
>> Bongjae Chang
>>
>>
>> ----- Original Message -----
>> From: "Oleksiy Stashok" <Oleksiy.Stashok_at_Sun.COM>
>> To: <dev_at_grizzly.dev.java.net>
>> Sent: Wednesday, July 01, 2009 6:11 PM
>> Subject: Re: Supporing OP_READ round robin on UDP
>>
>>
>>> Hi,
>>>
>>>>> I modified the proposed patch and attached it.
>>>>>
>>>>> As Alexey's words, I think that it is impossible that we support
>>>>> round robin SelectionKey distribution for UDP server-side.
>>>>>
>>>>> So I support UDP client-side connections distribution in the
>>>>> attached patch instead of server-side.
>>>>>
>>>>> In UDPConnectorHandler#connect(), I select a
>>>>> relativeSelectorHandler from aux controller. Then, client-side's
>>>>> OP_READ will be controlled in aux controller.
>>>>>
>>>>> When I used many UDPConnectorHandlers concurrently on 4 CPUs(local
>>>>> Windows machine), I found that the perf was improved. (About
>>>>> 30%~40%??, but it is not accurate)
>>>>>
>>>>> Could you please review this?
>>>>
>>>>
>>>> It seems to me that RoundRobinSelectorHandler does not distribute
>>>> the load
>>>> in a round robin manner. That is because roundRobinCounter is a
>>>> primitive
>>>> integer, so it seems that nextController method returns same
>>>> ReadController to different threads.
>>>>
>>>> I am not sure about this, but should we change roundRobinCounter to
>>>> volatile int, or AtomicInteger? This is not a big issue, though.
>>> Good catch!
>>> Though this issue could occur only, if we have more than 1 transport
>>> (SelectorHandler) per Controller, because otherwise only one
>>> SelectorRunner calls RRSelectorHandler, IMHO it makes sense to make
>>> counter atomic.
>>>
>>> Thank you.
>>>
>>> WBR,
>>> Alexey.
>>>
>>>>
>>>>
>>>> Minoru
>>>>
>>>>> And if you find some points which I have missed, please let me
>>>>> know.
>>>>>
>>>>> Thanks!
>>>>>
>>>>> --
>>>>> Bongjae Chang
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>> From: Bongjae Chang
>>>>> To: dev_at_grizzly.dev.java.net
>>>>> Sent: Monday, June 29, 2009 9:26 PM
>>>>> Subject: Re: Supporing OP_READ round robin on UDP
>>>>>
>>>>>
>>>>> Hi Alexey,
>>>>>
>>>>> I see. You are right!
>>>>>
>>>>> I was perhaps misunderstanding the issue. :(
>>>>>
>>>>> I will investigate it again.
>>>>>
>>>>> Thank you for your advice.
>>>>>
>>>>> --
>>>>> Bongjae Chang
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>> From: Oleksiy Stashok
>>>>> To: dev_at_grizzly.dev.java.net
>>>>> Sent: Monday, June 29, 2009 8:23 PM
>>>>> Subject: Re: Supporing OP_READ round robin on UDP
>>>>>
>>>>>
>>>>> Hi Bongjae,
>>>>>
>>>>>
>>>>> can you pls. describe the usecase, how we can support round
>>>>> robin SelectionKey distribution for UDP server-side? We usually
>>>>> open just one UDP server-side listener on specific port, which is
>>>>> represented by single DatagramChannel. So *all* the UDP client-side
>>>>> requests come just to *single* server-side DatagramChannel, so I'm
>>>>> not sure how we're able to distribute something here...
>>>>> On other side, it could be good idea to support UDP client-side
>>>>> connections distribution, when we use UDPConnectorHandlers.
>>>>>
>>>>>
>>>>> Thanks.
>>>>>
>>>>>
>>>>> WBR,
>>>>> Alexey.
>>>>>
>>>>>
>>>>> On Jun 27, 2009, at 7:39 , Bongjae Chang wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> I am investigating Grizzly issue #600( https://grizzly.dev.java.net/issues/show_bug.cgi?id=600
>>>>> , "OP_READ round robin in selector" ).
>>>>>
>>>>> And I attached the proposed patch.
>>>>>
>>>>> I tested all unit tests locally on 4 CPUs.
>>>>>
>>>>> But I couldn't do a performance test completely, so I think
>>>>> that more performance tests should be done.
>>>>>
>>>>> When I was going on the issue, I had a question.
>>>>>
>>>>> UDP is different from TCP because OP_READ is available without
>>>>> OP_ACCEPT in server side.
>>>>>
>>>>> When TCP receives OP_ACCEPT in main selector thread, TCP
>>>>> delivers OP_READ to auxiliary ReadController.
>>>>>
>>>>> It's OK.
>>>>>
>>>>> In my patch about UDP,
>>>>>
>>>>> I ignored a first OP_READ which was received in main selector
>>>>> thread for dispatching it, and registered it in auxiliary
>>>>> ReadController again like TCP.
>>>>>
>>>>> In other words, whenever UDP receives OP_READ directly in main
>>>>> selector thread, thread's context switch will occur for supporting
>>>>> round robin.
>>>>>
>>>>> But, if round robin on UDP is not supported, first OP_READ is
>>>>> handled in main selector thread without thread's context switch
>>>>> directly because of leader flollower strategy.
>>>>>
>>>>> So it is questionable whether UDP's round robin will improve
>>>>> the performance or not. But I believe that it will be able to
>>>>> improve the performance in some circumstances.
>>>>>
>>>>> If you have better idea for supporing round robin on UDP,
>>>>> please advice me.
>>>>>
>>>>> And please review the attached patch again.
>>>>>
>>>>> Thanks!
>>>>>
>>>>> --
>>>>> Bongjae Chang<patch.txt>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>>>>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>>>>>
>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>>>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>>>
>>>
>>>
>>>
>> <
>> patch
>> .txt
>> >
>> <
>> RoundRobinConnectionTest
>> .java
>> >---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>
>
>
>