users@grizzly.java.net

Re: TCPConnectorHandler

From: Jeanfrancois Arcand <Jeanfrancois.Arcand_at_Sun.COM>
Date: Thu, 31 May 2007 14:06:33 -0400

charlie hunt wrote:
> Jeanfrancois Arcand wrote:
>> charlie hunt wrote:
>>> Jeanfrancois Arcand wrote:
>>>> Salut,
>>>>
>>>> charlie hunt wrote:
>>>>> I'm not sure I agree with the suggested change. ;-)
>>>>>
>>>>> With 'nWrite == 0', you will reregister the key when no additional
>>>>> bytes are written. In other words, as long as more than 0 bytes
>>>>> have been written you'll continue to write. Only when it says it
>>>>> can't write any additional bytes will the key be re-registered.
>>>>>
>>>>> The main reason I'm disagreeing has to do with the potential cost
>>>>> of re-registering, parking & unparking a thread being much greater
>>>>> than spending a few more cpu cycles to keep writing as long as
>>>>> bytes are successfully written.
>>>>>
>>>>> Fwiw, it can take upwards of 50,000+ cpu cycles to go through the
>>>>> park / unpark.
>>>>>
>>>>> I think before blindly putting in the change I'd recommend we do
>>>>> some performance & scalability tests including some stress tests
>>>>> that have some really large ByteBuffer writes, (on the order of
>>>>> 100k sized ByteBuffer).
>>>>>
>>>>> Note: My argument is analogous to spinning on a mutex versus
>>>>> immediately sleeping when trying to obtain a lock. You will most
>>>>> often acquire the lock much quicker by spinning some number of
>>>>> cycles versus going to sleep and then being waken.
>>>>
>>>> One alternative is to loop over the socketChannel.write(bb) and exit
>>>> only when nWrite == 0 || !bb.remaining() like we are doing when we
>>>> use the temporary selector trick. What do you think?
>>>
>>> Yes, that may be a better alternative.
>>>
>>> I think which choice we make we should do some performance /
>>> scalability tests before deciding on which is the best approach.
>>>
>>> I think the thing(s) we want to avoid are:
>>> 1.) Force a thread to park & unpark to quickly
>>> 2.) Minimize the number of interactions with the Selector's
>>> SelectionKey's, (i.e. enabling / disabling interest ops).
>>>
>>> I think if we keep those two principles in mind in this case, I think
>>> we'll find the most optimal solution.
>>
>> Agree. But since we don't have the support for benchmarking this right
>> now (we need to automate faban runs inside Maven 2 so every run can
>> catch performance regression...I know this is doable :-)) I will go
>> ahead and commit the fix for now.
>>
>> Thanks!
>
> How about we start using the Project Grizzly IssueTracker ?

Agree! Can you file it with the info you just shared?

>
> Then, we can add as an "Enhancement" to automate faban runs.
>
> And, also add a "Task" to performance / scalability test this specific
> change with alternatives. I think it would be useful to cross-reference
> both issues with each other and possibly include part or all of this
> e-mail trail.
>
> What do you think?

+1 ... We already have 2 issues filled :-)

Thanks

-- Jeanfrancois

>
> charlie ...
>
>>
>> -- Jeanfrancois
>>
>>
>>>
>>> charlie ...
>>>
>>>>
>>>> -- Jeanfrancois
>>>>>
>>>>> charlie ...
>>>>>
>>>>> Jeanfrancois Arcand wrote:
>>>>>> Hi Karsten,
>>>>>>
>>>>>> Karsten Ohme wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> in TCPConnectorhandler in the read and write methods the key is
>>>>>>> only reattached if 0 is returned by the read and write method. It
>>>>>>> would make more sense if the key is reregistered if not all
>>>>>>> remaining bytes are written.
>>>>>>>
>>>>>>> instead of:
>>>>>>>
>>>>>>> if (nWrite == 0){
>>>>>>> key.attach(callbackHandler);
>>>>>>> selectorHandler.register(key,SelectionKey.OP_WRITE);
>>>>>>> }
>>>>>>> better:
>>>>>>>
>>>>>>> if (byteBuffer.hasRemaining()){
>>>>>>> key.attach(callbackHandler);
>>>>>>> selectorHandler.register(key,SelectionKey.OP_WRITE);
>>>>>>> }
>>>>>>
>>>>>> I agree. Will apply your patch.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> -- Jeanfrancois
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Regards,
>>>>>>> Karsten
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>>
>>>>>>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>>>>>>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>>>>>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>>>>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>>>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: users-help_at_grizzly.dev.java.net