Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
Hi Bozzo, Please take a look at PROTON-668 and confirm none of the assumptions are counter to your usage, especially whether you fit scenario 3 and do not have problems with the proposed restriction to pn_pipe. If this is OK for you, I think I am close to a fix that will work for you. Cliff On Wed, Sep 10, 2014 at 4:51 AM, Bozo Dragojevic wrote: > Hi Cliff, > > I agree that the current extra API call is kludgy if not downright ugly. > > I noticed today you already added a new API call pn_io_selector(), > which has separate windows and posix implementations. > > For short term 'fix', I'd propose to hide my hack inthere -- > make calling pn_io_selector() turn on the iocp on windows. > > Bozzo > > > This would not impact the API and would restore the > On 9. 09. 14 07:02, Cliff Jansen wrote: >> Ho Bozo, >> >> Thank you for comments and the suggested patch. I would prefer a >> solution that did not have a special Windows-only-sometimes call >> "pn_io_no_iocp()". It seems to me anyway that there is another class >> of sockets that are pulled into an IOCP context too early, so that a >> separate solution is required that should fix your problem too. >> Basically, a more lazy enlistment strategy should leave you outside >> IOCP and fix the other issue too. >> >> Based on your problem and how Dispatch strives for multithreaded >> performance, I think I have a better handle on what Proton should be >> providing for small to medium-large scalability. >> >> I am going to try to define what is a sensible intersection of Windows >> and Posix capabilities to be supported by the proton >> io/selector/selectable classes in a separate documentation JIRA and >> try to get a fix for you ASAP, probably in yet another JIRA. >> >> Cliff >
Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
Hi Cliff, I agree that the current extra API call is kludgy if not downright ugly. I noticed today you already added a new API call pn_io_selector(), which has separate windows and posix implementations. For short term 'fix', I'd propose to hide my hack inthere -- make calling pn_io_selector() turn on the iocp on windows. Bozzo This would not impact the API and would restore the On 9. 09. 14 07:02, Cliff Jansen wrote: > Ho Bozo, > > Thank you for comments and the suggested patch. I would prefer a > solution that did not have a special Windows-only-sometimes call > "pn_io_no_iocp()". It seems to me anyway that there is another class > of sockets that are pulled into an IOCP context too early, so that a > separate solution is required that should fix your problem too. > Basically, a more lazy enlistment strategy should leave you outside > IOCP and fix the other issue too. > > Based on your problem and how Dispatch strives for multithreaded > performance, I think I have a better handle on what Proton should be > providing for small to medium-large scalability. > > I am going to try to define what is a sensible intersection of Windows > and Posix capabilities to be supported by the proton > io/selector/selectable classes in a separate documentation JIRA and > try to get a fix for you ASAP, probably in yet another JIRA. > > Cliff
Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
Ho Bozo,
Thank you for comments and the suggested patch. I would prefer a
solution that did not have a special Windows-only-sometimes call
"pn_io_no_iocp()". It seems to me anyway that there is another class
of sockets that are pulled into an IOCP context too early, so that a
separate solution is required that should fix your problem too.
Basically, a more lazy enlistment strategy should leave you outside
IOCP and fix the other issue too.
Based on your problem and how Dispatch strives for multithreaded
performance, I think I have a better handle on what Proton should be
providing for small to medium-large scalability.
I am going to try to define what is a sensible intersection of Windows
and Posix capabilities to be supported by the proton
io/selector/selectable classes in a separate documentation JIRA and
try to get a fix for you ASAP, probably in yet another JIRA.
Cliff
On Fri, Sep 5, 2014 at 3:18 AM, Bozo Dragojevic wrote:
> On 5. 09. 14 11:53, Bozo Dragojevic wrote:
>>
>>
>> The patch is a bit rough
>> - not sure I really like the name pn_io_no_iocp()
>> - missing non-windows stub for pn_io_no_iocp()
>> - it most likely handles the selector access wrong (see XXX comment)
>
> I missed this bit (blush)
>
> diff --git a/proton-c/src/windows/io.c b/proton-c/src/windows/io.c
> index f9c85b4..b5fc7b2 100644
> --- a/proton-c/src/windows/io.c
> +++ b/proton-c/src/windows/io.c
> @@ -263,6 +263,14 @@ pn_socket_t pn_connect(pn_io_t *io, const char
> *hostarg, const char *port)
>if (io->iocp != NULL) {
> sock = pni_iocp_begin_connect(io->iocp, sock, addr, io->error);
>} else {
> +if (connect(sock, addr->ai_addr, addr->ai_addrlen) != 0) {
> + if (WSAGetLastError() != WSAEWOULDBLOCK) {
> +pni_win32_error(io->error, "connect", WSAGetLastError());
> +freeaddrinfo(addr);
> +closesocket(sock);
> +return INVALID_SOCKET;
> + }
> +}
> freeaddrinfo(addr);
>}
>return sock;
>
> Bozzo
Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
On 5. 09. 14 11:53, Bozo Dragojevic wrote:
The patch is a bit rough
- not sure I really like the name pn_io_no_iocp()
- missing non-windows stub for pn_io_no_iocp()
- it most likely handles the selector access wrong (see XXX comment)
I missed this bit (blush)
diff --git a/proton-c/src/windows/io.c b/proton-c/src/windows/io.c
index f9c85b4..b5fc7b2 100644
--- a/proton-c/src/windows/io.c
+++ b/proton-c/src/windows/io.c
@@ -263,6 +263,14 @@ pn_socket_t pn_connect(pn_io_t *io, const char
*hostarg, const char *port)
if (io->iocp != NULL) {
sock = pni_iocp_begin_connect(io->iocp, sock, addr, io->error);
} else {
+if (connect(sock, addr->ai_addr, addr->ai_addrlen) != 0) {
+ if (WSAGetLastError() != WSAEWOULDBLOCK) {
+pni_win32_error(io->error, "connect", WSAGetLastError());
+freeaddrinfo(addr);
+closesocket(sock);
+return INVALID_SOCKET;
+ }
+}
freeaddrinfo(addr);
}
return sock;
Bozzo
Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
Hi Cliff,
On 4. 09. 14 20:03, Cliff Jansen wrote:
Hi,
I am not sure what you are trying to do with xdispatch.
Exactly the same idea as contrib/proton-hawtdispatch, but in C.
If you are
using and external loop and managing your own IO, you can bypass the
low level Proton IO primitives and access the proton engine directly.
This is how Qpid uses Proton.
I am using the engine directly.
I am also using io.[ch] as a *very* convenient multi platform socket
abstraction.
If you were using the driver.c mechanism, please take a look at
PROTON-658. This and the main messenger loop show how the
selectors/selectables can be used within Proton. pn_selector_select()
is pretty central to the completion port model on windows.
I'm not using the proton driver, I have my own driver and thread model.
All serialization of one engine instance is one serial libdispatch task
queue.
I'm also not using proton selectors but two libdispatch sources (read
and write)
that are bound to the same task queue.
All bits of my application that need to be talk to the engine then just
queue
tasks to the same libdispatch queue, so we have practically no locks in
the app.
You may find that if you use Windows accept and connect instead of
pn_accept and pn_connect you can keep your descriptors completely
outside the completion port model and use things as you did. If not,
perhaps we can find another way to isolate your sockets from the
completion port implementation.
I like io's eror thing, and the wouldblock thing, I'd hate to duplicate
all that
across the platforms.
Today I had a read through the iocp change.
I see it kept support for 'user supplied' sockets, where it behaves
synchronously, as before.
I can make iocp support be controlled by an opt-out parameter.
Then for example, pn_listen does not enroll the socket to iocp and
pn_accept
does not find it in the map and treat it as user supplied.
Patch attached, with it my code works again.
Would such a change be acceptable?
The patch is a bit rough
- not sure I really like the name pn_io_no_iocp()
- missing non-windows stub for pn_io_no_iocp()
- it most likely handles the selector access wrong (see XXX comment)
The Windows completion port implementation is designed to provide a
highly scalable throughput in a constrained (mostly) single thread
model, but remain lightweight for small clients. Central to that is
the integration with selectors and selectables. But it can't be
everything to everybody.
I did not want to say this is a bad thing! :) I just wanted to let you
guys know
there is a user of the library with slightly different use-case and to
figure out if
such use can be considered to be within scope of the things that proton
is trying to accomplish.
Cheers,
Bozzo
From 357bf8fa698e9a49170b5e53b9abb2a74007dbb2 Mon Sep 17 00:00:00 2001
From: Bozo Dragojevic
Date: Fri, 5 Sep 2014 11:40:57 +0200
Subject: [PATCH 1/1] Make iocp optional on windows
---
proton-c/include/proton/io.h | 1 +
proton-c/src/windows/io.c| 47 ++--
proton-c/src/windows/iocp.c | 8
proton-c/src/windows/iocp.h | 1 +
4 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/proton-c/include/proton/io.h b/proton-c/include/proton/io.h
index 2d56736..3aac524 100644
--- a/proton-c/include/proton/io.h
+++ b/proton-c/include/proton/io.h
@@ -47,6 +47,7 @@ typedef struct pn_io_t pn_io_t;
typedef struct pn_selector_t pn_selector_t;
PN_EXTERN pn_io_t *pn_io(void);
+PN_EXTERN bool pn_io_no_iocp(pn_io_t *io);
PN_EXTERN void pn_io_free(pn_io_t *io);
PN_EXTERN pn_error_t *pn_io_error(pn_io_t *io);
PN_EXTERN pn_socket_t pn_connect(pn_io_t *io, const char *host, const char
*port);
diff --git a/proton-c/src/windows/io.c b/proton-c/src/windows/io.c
index b5660be..f9c85b4 100644
--- a/proton-c/src/windows/io.c
+++ b/proton-c/src/windows/io.c
@@ -92,7 +92,8 @@ void pn_io_finalize(void *obj)
{
pn_io_t *io = (pn_io_t *) obj;
pn_error_free(io->error);
- pn_free(io->iocp);
+ if (io->iocp != NULL)
+pn_free(io->iocp);
WSACleanup();
}
@@ -112,6 +113,20 @@ void pn_io_free(pn_io_t *io)
pn_free(io);
}
+bool pn_io_no_iocp(pn_io_t *io)
+{
+ if (io->iocp != NULL) {
+if (pni_iocpdesc_map_empty(io->iocp)) {
+ pn_free(io->iocp);
+ io->iocp = NULL;
+} else {
+ pn_error_format(io->error, PN_ERR, "Cannot turn off iocp, already in
use");
+ return false;
+}
+ }
+ return true;
+}
+
pn_error_t *pn_io_error(pn_io_t *io)
{
assert(io);
@@ -210,14 +225,17 @@ pn_socket_t pn_listen(pn_io_t *io, const char *host,
const char *port)
return INVALID_SOCKET;
}
- iocpdesc_t *iocpd = pni_iocpdesc_create(io->iocp, sock, false);
- if (!iocpd) {
-pn_i_error_from_errno(io->error, "register");
-closesocket(sock);
-return INVALID_SOCKET;
+ if (io->iocp != NULL) {
+iocpdesc_t *iocpd = pni_iocpdesc_create(io->iocp, sock, false);
+if (!iocpd) {
+ pn_i_error_
Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
Hi Bozo, I closed the JIRA before I noticed your issue. Please feel free to reopen it if your investigations turns up any problems with it. --Rafael On Thu, Sep 4, 2014 at 11:33 AM, Bozo Dragojevic wrote: > Hi! > > Just a headsup. This commit breaks pn_accept() for me, it fails all the > time. I'm not using selectors but drive the socket > with libdispatch (from xdispatch) with a custom driver. > > If I revert this commit the trunk is useable. I'll dig deeper what is > going on, I was focusing on getting SSL to work on windows. > > Cheers, > Bozzo > > > On 4. 09. 14 04:12, ASF subversion and git services (JIRA) wrote: > >> [ https://issues.apache.org/jira/browse/PROTON-640?page= >> com.atlassian.jira.plugin.system.issuetabpanels:comment- >> tabpanel&focusedCommentId=14120849#comment-14120849 ] >> >> ASF subversion and git services commented on PROTON-640: >> >> >> Commit 1622394 from cliffjan...@apache.org in branch 'proton/trunk' >> [ https://svn.apache.org/r1622394 ] >> >> PROTON-640 Windows IO completion port implementation for pn_io and >> pn_selectable classes >> >> IO completion port Windows IO for Proton >>> >>> >>> Key: PROTON-640 >>> URL: https://issues.apache.org/jira/browse/PROTON-640 >>> Project: Qpid Proton >>> Issue Type: Improvement >>> Components: proton-c >>> Affects Versions: 0.7 >>> Environment: Windows >>> Reporter: Cliff Jansen >>> Assignee: Cliff Jansen >>> Fix For: 0.8 >>> >>> >>> Provide a native IO completion port layer similar to the C++ QPID >>> version for Proton. >>> >> >> >> -- >> This message was sent by Atlassian JIRA >> (v6.3.4#6332) >> > >
Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
Hi, I am not sure what you are trying to do with xdispatch. If you are using and external loop and managing your own IO, you can bypass the low level Proton IO primitives and access the proton engine directly. This is how Qpid uses Proton. If you were using the driver.c mechanism, please take a look at PROTON-658. This and the main messenger loop show how the selectors/selectables can be used within Proton. pn_selector_select() is pretty central to the completion port model on windows. You may find that if you use Windows accept and connect instead of pn_accept and pn_connect you can keep your descriptors completely outside the completion port model and use things as you did. If not, perhaps we can find another way to isolate your sockets from the completion port implementation. The Windows completion port implementation is designed to provide a highly scalable throughput in a constrained (mostly) single thread model, but remain lightweight for small clients. Central to that is the integration with selectors and selectables. But it can't be everything to everybody. On Thu, Sep 4, 2014 at 8:33 AM, Bozo Dragojevic wrote: > Hi! > > Just a headsup. This commit breaks pn_accept() for me, it fails all the > time. I'm not using selectors but drive the socket > with libdispatch (from xdispatch) with a custom driver. > > If I revert this commit the trunk is useable. I'll dig deeper what is going > on, I was focusing on getting SSL to work on windows. > > Cheers, > Bozzo > > > On 4. 09. 14 04:12, ASF subversion and git services (JIRA) wrote: >> >> [ >> https://issues.apache.org/jira/browse/PROTON-640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14120849#comment-14120849 >> ] >> >> ASF subversion and git services commented on PROTON-640: >> >> >> Commit 1622394 from cliffjan...@apache.org in branch 'proton/trunk' >> [ https://svn.apache.org/r1622394 ] >> >> PROTON-640 Windows IO completion port implementation for pn_io and >> pn_selectable classes >> >>> IO completion port Windows IO for Proton >>> >>> >>> Key: PROTON-640 >>> URL: https://issues.apache.org/jira/browse/PROTON-640 >>> Project: Qpid Proton >>> Issue Type: Improvement >>> Components: proton-c >>> Affects Versions: 0.7 >>> Environment: Windows >>> Reporter: Cliff Jansen >>> Assignee: Cliff Jansen >>> Fix For: 0.8 >>> >>> >>> Provide a native IO completion port layer similar to the C++ QPID version >>> for Proton. >> >> >> >> -- >> This message was sent by Atlassian JIRA >> (v6.3.4#6332) > >
Re: [jira] [Commented] (PROTON-640) IO completion port Windows IO for Proton
Hi! Just a headsup. This commit breaks pn_accept() for me, it fails all the time. I'm not using selectors but drive the socket with libdispatch (from xdispatch) with a custom driver. If I revert this commit the trunk is useable. I'll dig deeper what is going on, I was focusing on getting SSL to work on windows. Cheers, Bozzo On 4. 09. 14 04:12, ASF subversion and git services (JIRA) wrote: [ https://issues.apache.org/jira/browse/PROTON-640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14120849#comment-14120849 ] ASF subversion and git services commented on PROTON-640: Commit 1622394 from cliffjan...@apache.org in branch 'proton/trunk' [ https://svn.apache.org/r1622394 ] PROTON-640 Windows IO completion port implementation for pn_io and pn_selectable classes IO completion port Windows IO for Proton Key: PROTON-640 URL: https://issues.apache.org/jira/browse/PROTON-640 Project: Qpid Proton Issue Type: Improvement Components: proton-c Affects Versions: 0.7 Environment: Windows Reporter: Cliff Jansen Assignee: Cliff Jansen Fix For: 0.8 Provide a native IO completion port layer similar to the C++ QPID version for Proton. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
