On Fri, Mar 24, 2017 at 9:10 AM, Vincent Torri <[email protected]> wrote:
> On Fri, Mar 24, 2017 at 7:45 AM, Gustavo Sverzut Barbieri
> <[email protected]> wrote:
>> On Fri, Mar 24, 2017 at 1:44 AM, Vincent Torri <[email protected]> 
>> wrote:
>>> On Thu, Mar 23, 2017 at 4:44 PM, Gustavo Sverzut Barbieri
>>> <[email protected]> wrote:
>>>> On Thu, Mar 23, 2017 at 9:45 AM, Vincent Torri <[email protected]> 
>>>> wrote:
>>>>> if i have read the code correctly, there are problems:
>>>>>
>>>>> Efl_Net_Socket_Windows_Operation is an analogous of my IRP but you're
>>>>> wrong in modifying it, this will not work
>>>>
>>>> elaborate on that please? How is it not working? The relevant change is:
>>>>
>>>> your code used a enum (op), then a single "complete cb" that would use
>>>> a "switch" on that op... then switch on errors... I changed that to 2
>>>> callbacks, on non-errors, call the success callback, on failure call
>>>> the error callback. Your single callback with nested switch is
>>>> replaced my multiple callbacks, one for each scope.
>>>>
>>>> please elaborate more on why is it wrong and what could go wrong.
>>>
>>> my single callback, that is  _server|client_on_complete(),  is
>>> responsible of ALL the I/O requests, that is client connection, read
>>> and write  in our case. There must be one callback for these
>>> operations, that's how the I/O completion port with thread pool is
>>> working.
>>
>> see that the IOCP callback that is called from thread pool is still a
>> single one, however it dispatches to given callbacks, instead of using
>> an enum, maybe you missed that part?
>>
>>> you removed all the 'case's for each operation (connection, read). Did
>>> you remak that break is not always called for each 'case' ? You have
>>> broken the logic here.
>>
>> I may have missed some, indeed. I'll check with the MSDN docs on the
>> behavior, as well as your code.
>>
>>
>>>>> my error_check is not correctly ported
>>>>
>>>> again, elaborate on that.
>>>
>>> do you free the IRP ?
>>
>> of course, it's done by operation_done(), it will call back the user
>> (failure or success), then free the IRP
>> (Efl_Net_Socket_Windows_Operation).
>>
>>
>>> Btw, CreateThreadPoolIO() must be called each time a server (resp.
>>> client) instance is created (that is when CreateNamedPipe() (resp.
>>> CreateFile()) is called.
>>
>> This is done automatically, look at efl_net_socket_windows_init(),
>> this function is called by both server and client to adopt a HANDLE,
>> then the CreateThreadPoolIo() is called.
>>
>> I'm going to sleep now, but I used my whole day reading all the
>> Windows documentation about overlapped I/O, IOCP and the new helper
>> using thread pools on top, that you used.
>>
>> Basically it's all on top of overlapped primitives, with IOCP being an
>> "easy way" to wait for completion on a thread... but once you call the
>> get queued events, your thread is bound to that, then you need one
>> thread per IOCP... To make that easier they created a wrapper on top
>> of thread pool that creates an IOCP for that thread and add multiple
>> HANDLE to it, calling you back later. They claim more performance due
>> enhanced concurrency.
>>
>> So far, so good. But given how the rest of EFL works, the enhanced
>> concurrency and efficiency won't matter much, we're basically single
>> threaded. So while the ThreadpoolIo will speed up the overlapped
>> operation queuing... we're not queuing stuff, since we only do that
>> from main loop and we'd need to go back from thread to main loop, call
>> the user and let him do a new request that originates a new overlapped
>> I/O.
>
>> TL;DR: we'll never benefit from the thread pool benefits.
>
> it will if we have a lot of use of connections using ecore_con_local
> or ecore_ipc

also, it does not consume HANDLE's, contrary to what you will use with
the design you want to choose (which is btw my first implementation of
ecore_con_local, you're re-writing it...) and will be faster. The
Windows port is already largely slower than the linux one. I've chosen
I/O completion port for that very reason

Vincent


>> Since it is a "new" API that is not available in Wine,
>
> you told me to use a new API and forget support of Windows XP. It's
> not surprising that Wine does not support such new API...
>
>> I kept reading
>> and found something that is usable with Wine and that doesn't hurt our
>> performance that much, actually two options:
>
> no, another one.. My first attemp, using BindIOCompletionCallback(),
> supported by Windows XP and Wine, which is almost the same
> implementation than using the thread pool IO
>
>>
>>  - OVERLAPPED.hEvent + ecore_main_win32_handler_add(): with this, once
>> the overlapped I/O completes, it will wake up the main loop and call
>> you back. No changes to Ecore are needed and we work with wine.
>>
>>  - ReadFileEx() and WriteFileEx()... these would work if we used
>> "alertable" wait calls (those ending with "Ex"), but we don't. Would
>> require changes to ecore_main.c and we'd still miss the solution for
>> ConnectNamedPipe in servers.
>>
>> Given these, I'm experimenting with the first option:
>> OVERLAPPED.hEvent + ecore_main_win32_handler_add()... I managed to run
>> that in wine and find many integration bugs (like missing "\" in the
>> path... and many others). But I can run a server and a client, they
>> communicate. Some stuff is broken and there are way too many
>> debug/printf which I plan to remove... I expect late friday I'll have
>> something concrete/usable to send for review.
>
>
> ok, gook luck
>
> Vincent

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to