Karsten Ohme wrote:
> On Thu, May 31, 2007 at 01:16:00PM -0500, charlie hunt wrote:
>> 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);
>> }
>
>
> This is busy waiting. It does not differ from the blocking case (?) What happens, if the channel does not allow to transmit data for a longer period or if the
> given buffer is very large? Does it
> block the selector?
Now it won't block. If the client isn't ready the bytes, the
socketChannel.write() will return 0 and we will register back the
OP_WRITE. Once the client has cleared its buffer (or the network is
ready for more write), the Selector will call back your CallbackHandler.
OK, I would do something similar in my end application to get rid of the
data, but there may be an application which which uses the time
> in the while loop to do some background jobs.
It will block only if you use the temporary selector trick
(write(bb,true)). I don't think you will face issue using the non
blocking one, unless I'm missing something.
Thanks
-- Jeanfrancois
>
> Karsten
>
>> 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/>
>>
>
>> begin:vcard
>> fn:charlie hunt
>> n:hunt;charlie
>> email;internet:charlie.hunt_at_sun.com
>> tel;work:x47708 or (630) 285-7708
>> x-mozilla-html:TRUE
>> version:2.1
>> end:vcard
>>
>>
>
>> ---------------------------------------------------------------------
>> 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
>