Looks good for me!
Thank you very much.
WBR,
Alexey.
On Jul 1, 2009, at 16:04 , Bongjae Chang wrote:
> Thanks Alexey,
>
> I modified the patch.
>
> I removed the meaningless code in SelectorHandlerRunner.java and
> RoundRobinSelectorHandler.java.
>
> (SelectorHandlerRunner.java and ReadController.java was not changed
> and was reverted from current svn)
>
> If any codes should be modified more, please let me know.
>
> 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 10:44 PM
> Subject: Re: Supporing OP_READ round robin on UDP
>
>
>> Hi Bongjae,
>>
>>> 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?
>> Yes, it's what I meant :)
>>
>> Thank you.
>>
>> WBR,
>> Alexey.
>>
>>
>>>
>>> --
>>> 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
>>>>
>>>>
>>>>
>>>>
>>
>>
>> ---------------------------------------------------------------------
>> 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
> >---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net