>> And when preparing to notify CallbackHandler, TCPSelectorHandler
>> checks if callbackHandler implements CallbackHandlerDescriptor
>> interface, if yes - checks if current CallbackHandler operation
>> should be run in current or separate thread.
>> This way, from one side existing apps will work the same way there
>> worked, and we will have ability control CallbackHandler method
>> invocation in cases, where we will need that.
>
> OK looks good. I'm just afraid of the performance of invoking
> instanceof on every operations :-(, but I don't have a better idea to
> propose....
Not sure, IMHO it's fast enough. May be Charlie or Scott is aware?
Thanks.
WBR,
Alexey.
>
> +1
>
> -- Jeanfrancois
>
>
>
>>
>> Thanks.
>>
>> WBR,
>> Alexey.
>>
>> Jeanfrancois Arcand wrote:
>>> Salut,
>>>
>>> Oleksiy Stashok wrote:
>>>> Hi,
>>>>
>>>>>>>> think it could be good idea from perf. side, but only if it
>>>>>>>> will be used right way.
>>>>>>>>
>>>>>>>> We have to be very careful with that change! Because if someone
>>>>>>>> will have complex logic in CallbackHandler.onConnect - it will
>>>>>>>> lead to big perf. problems.
>>>>>>>> For example - SSLConnectorHandler usage. Currently, in our
>>>>>>>> samples from time to time we execute *blocking*
>>>>>>>> sslConnectorHandler.handshake directly from onConnect method,
>>>>>>>> which definitely should not happen anymore!
>>>>>>>
>>>>>>> Agree this is not good for TLS as this operations is quite slow.
>>>>>>> I would think we should made it configurable then, and let the
>>>>>>> current default to spawn a thread for TLS, but not for
>>>>>>> TCP....but that might be quite confusing. Not sure what is the
>>>>>>> best solution...
>>>>>> What about making current mode as default, and make it
>>>>>> configurable by adding
>>>>>> TCPSelectorHandler.is/setConnectOnSeparateThread() ?
>>>>>> Because I'm not sure if it will be more clear, if we will have
>>>>>> different default behaviors for TCP and SSL.
>>>>>>
>>>>>> What do you think?
>>>>>
>>>>> OK make sense. What about UDPSelectorHandler? I would think we
>>>>> need to promote the method on the ConnectorHandler so both
>>>>> implementation can turn it on/off
>>>> With UDPSelectorHandler we have different situation. Currently our
>>>> UDP implementation tries to behave the same or almost the same way
>>>> TCP does. But actually for UDP we don't need all that connect
>>>> stuff. As UDP channel is connected just after
>>>> DatagramChannel.connect(..) is called. So for UDP there is no need
>>>> at all in CallbackHandler.onConnect(...).
>>>> On other side, after executing UDPConnectorHandler.connect(...) we
>>>> can not be sure that datagram channel is registered on selector, we
>>>> can just schedule registration task, which will be executed on
>>>> selector thread.
>>>> But is it required for us to be notified when datagram channel
>>>> becomes registered with selector? Think for most of scenarios this
>>>> info is not required.
>>>
>>> OK so then what about adding a new interface that only have
>>> onRead/onWrite method and make CallbackHandler extending that
>>> interface with the onConnect method (so current applications aren't
>>> broken/still compatible) and handle the case inside the
>>> SelectorHandler? I think we can extends both TCP and UDP, where for
>>> TCP we do the minimal operations like registering for OP_READ and
>>> OP_WRITE?
>>>
>>>
>>>> If we will remove TCP-like connect behavior from UDP implementation
>>>> - we can improve performance, as we will not need to redundant
>>>> context switches and return connected UDPConnectorHandler instantly.
>>>>
>>>> What do you think?
>>>
>>> +1.
>>>
>>> Let me work on some real implementation :-)
>>>
>>> A+
>>>
>>> -- jeanfrancois
>>>
>>>
>>>>
>>>> WBR,
>>>> Alexey.
>>>>>
>>>>> A+
>>>>>
>>>>> -- jeanfrancois
>>>>>
>>>>>>
>>>>>> WBR,
>>>>>> Alexey.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> In our javadoc we should warn about such *new default* behavior
>>>>>>>> :))
>>>>>>>>
>>>>>>>> BTW, We can try to implement this more common way (attached)? :)
>>>>>>>
>>>>>>> LOL :-) Yes I prefer your patch :-) What about exposing the
>>>>>>> runInSeparateThread properties so it can be changed directly
>>>>>>> when using the SelectorHandler? The bad side is it will require
>>>>>>> bumping the release version as we are introducing a new API....
>>>>>>>
>>>>>>> A+
>>>>>>>
>>>>>>> -- Jeanfrancois
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>> WBR,
>>>>>>>> Alexey.
>>>>>>>>
>>>>>>>> Jeanfrancois Arcand wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> we recently faced a bug in Sailfin where performance
>>>>>>>>> significantly decreased (cut&paste from Scott update on the bug):
>>>>>>>>>
>>>>>>>>>> This is not so much lock contention as it is a form of
>>>>>>>>>> deadlock that happens
>>>>>>>>>> under load. What happens is that all the SipContainer threads
>>>>>>>>>> attempt to write
>>>>>>>>>> to a socket (the sipp uas process); one of them will open the
>>>>>>>>>> socket while the
>>>>>>>>>> rest of them block waiting to enter the same code. The thread
>>>>>>>>>> that opens a
>>>>>>>>>> socket then waits for the socket connection to be completed.
>>>>>>>>>>
>>>>>>>>>> The completion of the socket connection is signaled by the
>>>>>>>>>> TCPSelectorThread,
>>>>>>>>>> which creates a callback task and executes it on the
>>>>>>>>>> SipContainer thread pool.
>>>>>>>>>> Since all of those threads are busy (blocked waiting to enter
>>>>>>>>>> the socket-writing
>>>>>>>>>> code), the callback isn't ever processed, and the stack trace
>>>>>>>>>> below results.
>>>>>>>>>> Eventually, the thread waiting for the callback will time
>>>>>>>>>> out, and things will
>>>>>>>>>> run for a few seconds until the same state is reached (and
>>>>>>>>>> eventually, there
>>>>>>>>>> will be sockets created and things will run okay).
>>>>>>>>>>
>>>>>>>>>> A possible fix for sailing is to override the
>>>>>>>>>> TCPSelectorThread and execute the
>>>>>>>>>> onConnect callback directly rather than putting that on
>>>>>>>>>> another thread.
>>>>>>>>>
>>>>>>>>> I would like to propose that we change the current behavior
>>>>>>>>> and avoid executing the OP_CONNECT on its own thread, but
>>>>>>>>> instead executing using the SelectorHandler thread. This is
>>>>>>>>> what we are already doing for OP_ACCEPT based on some
>>>>>>>>> performance measure we did for Grizzly 1.0, and based on
>>>>>>>>> Scott's investigation, I would like to change the default
>>>>>>>>> behavior
>>>>>>>>>
>>>>>>>>>> Index: TCPSelectorHandler.java
>>>>>>>>>> ===================================================================
>>>>>>>>>>
>>>>>>>>>> --- TCPSelectorHandler.java (revision 712)
>>>>>>>>>> +++ TCPSelectorHandler.java (working copy)
>>>>>>>>>> @@ -697,8 +697,13 @@
>>>>>>>>>> try {
>>>>>>>>>> CallbackHandlerContextTask task =
>>>>>>>>>> CallbackHandlerContextTask.poll();
>>>>>>>>>> task.setCallBackHandler(callbackHandler);
>>>>>>>>>> - context.execute(task);
>>>>>>>>>> - } catch (PipelineFullException ex){
>>>>>>>>>> + if (context.getCurrentOpType() !=
>>>>>>>>>> Context.OpType.OP_CONNECT){
>>>>>>>>>> + context.execute(task);
>>>>>>>>>> + } else {
>>>>>>>>>> + task.setContext(context);
>>>>>>>>>> + task.call();
>>>>>>>>>> + }
>>>>>>>>>> + } catch (Exception ex){
>>>>>>>>>> throw new IOException(ex.getMessage());
>>>>>>>>>> }
>>>>>>>>>> }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If someone rely on the current behavior (spawning a Thread),
>>>>>>>>> then the TCPSelectorHandler can always be extended to act like
>>>>>>>>> previous version.
>>>>>>>>>
>>>>>>>>> I'm interested to learn from the Corba team about how
>>>>>>>>> OP_CONNECT has been handled (if that was used).
>>>>>>>>>
>>>>>>>>> A+
>>>>>>>>>
>>>>>>>>> -- Jeanfrancois
>>>>>>>>>
>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>>
>>>>>> 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
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>