You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Cliff Jansen <cl...@gmail.com> on 2012/01/04 21:12:57 UTC

Review Request: QPID-3571 Portable Poller implememtation for Posix systems

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3386/
-----------------------------------------------------------

Review request for qpid, Andrew Stitcher, Ted Ross, and Steve Huston.


Summary
-------

Portable qpid::sys::Poller implementation for Posix systems.

Modelled as closely as possible to the EpollPoller implementation.


This addresses bug QPID-3571.
    https://issues.apache.org/jira/browse/QPID-3571


Diffs
-----

  /svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/posix/PosixPoller.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/3386/diff


Testing
-------

linux

solaris (x86, sparc, gcc, native sunstudio compilers)


Thanks,

Cliff


Re: Review Request: QPID-3571 Portable Poller implememtation for Posix systems

Posted by Cliff Jansen <cl...@gmail.com>.

> On 2012-01-05 00:41:30, Steve Huston wrote:
> > Nit... there are a few trailing spaces, noted by red in the review diff.
> > 
> > There's a qpid/sys/PipeHandle.h that you may be able to use in the SignalPipe class, though there may not be too much benefit to that.
> > 
> > In EventStream::next, does the wait on serializer decrease the timeout? If not, the wait may end up longer than the caller requested.
> > 
> > Other than that, it's good!

Steve, thanks very much for your time reviewing this code... several times!

I will fix the spaces :-)

The timeout (qpid::sys::Duration) is converted to an absolute time (targetTimeout) prior to entering the loop.  The targetTimeout does not need further adjusting from within the loop.

The pre-existing PipeHandle class exposes read and write calls on the pipe whereas the poller's SignalPipe hides them (in order to minimize them).  The remaining overlap between them is very small, so I don't think there is much to be gained by trying to re-jig either one.


- Cliff


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3386/#review4192
-----------------------------------------------------------


On 2012-01-04 20:12:57, Cliff Jansen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3386/
> -----------------------------------------------------------
> 
> (Updated 2012-01-04 20:12:57)
> 
> 
> Review request for qpid, Andrew Stitcher, Ted Ross, and Steve Huston.
> 
> 
> Summary
> -------
> 
> Portable qpid::sys::Poller implementation for Posix systems.
> 
> Modelled as closely as possible to the EpollPoller implementation.
> 
> 
> This addresses bug QPID-3571.
>     https://issues.apache.org/jira/browse/QPID-3571
> 
> 
> Diffs
> -----
> 
>   /svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/posix/PosixPoller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3386/diff
> 
> 
> Testing
> -------
> 
> linux
> 
> solaris (x86, sparc, gcc, native sunstudio compilers)
> 
> 
> Thanks,
> 
> Cliff
> 
>


Re: Review Request: QPID-3571 Portable Poller implememtation for Posix systems

Posted by Steve Huston <sh...@riverace.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3386/#review4192
-----------------------------------------------------------


Nit... there are a few trailing spaces, noted by red in the review diff.

There's a qpid/sys/PipeHandle.h that you may be able to use in the SignalPipe class, though there may not be too much benefit to that.

In EventStream::next, does the wait on serializer decrease the timeout? If not, the wait may end up longer than the caller requested.

Other than that, it's good!

- Steve


On 2012-01-04 20:12:57, Cliff Jansen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3386/
> -----------------------------------------------------------
> 
> (Updated 2012-01-04 20:12:57)
> 
> 
> Review request for qpid, Andrew Stitcher, Ted Ross, and Steve Huston.
> 
> 
> Summary
> -------
> 
> Portable qpid::sys::Poller implementation for Posix systems.
> 
> Modelled as closely as possible to the EpollPoller implementation.
> 
> 
> This addresses bug QPID-3571.
>     https://issues.apache.org/jira/browse/QPID-3571
> 
> 
> Diffs
> -----
> 
>   /svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/posix/PosixPoller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3386/diff
> 
> 
> Testing
> -------
> 
> linux
> 
> solaris (x86, sparc, gcc, native sunstudio compilers)
> 
> 
> Thanks,
> 
> Cliff
> 
>