users@grizzly.java.net

Re: TCPConnectorHandler

From: charlie hunt <charlie.hunt_at_sun.com>
Date: Thu, 31 May 2007 13:16:00 -0500

Jeanfrancois Arcand wrote:
> 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?

Yes ... will do during my conference calls this afternoon.

Btw, I think the commit you just did will likely be the best performing,
(but, we'll find out in due time).

For those who missed Jeanfrancois's commit:

+ int nWrite = 1;
+ while (nWrite > 0 && byteBuffer.hasRemaining()){
+ nWrite = socketChannel.write(byteBuffer);
+ }
+
+ if (nWrite == 0 && byteBuffer.hasRemaining()){
                 key.attach(callbackHandler);
                 selectorHandler.register(key,SelectionKey.OP_WRITE);
             }

charlie ...

>
>>
>> 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
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>

-- 
Charlie Hunt
Java Performance Engineer
630.285.7708 x47708 (Internal)
<http://java.sun.com/docs/performance/>