Re: [qpid-proton-cpp] default_container does not implement thread safety at all?

[Fj <fj...@gmail.com>]

By the way, if anyone is interested, as a stop-gap solution I ended up just
implementing a timer-based callback reenqueuing itself every 50ms and
pulling callbacks from a properly synchronized queue and executing them on
the worker thread. It's not even that bad performance-wise, it just adds
those 50/2 ms of latency on average.

A better solution would be to do the same thing the Python binding does,
expose a Selectable interface and use it to wake up the reactor thread.

And by the way there's a thing that you might want to consider: there's a
bunch of use cases that don't need inter-thread synchronization, for
example, single-threaded workers, or multi-threaded workers that only need
to know when to stop and they know it from the socket being closed. In
those cases exposing a thread-safe Container.inject(callback) is a total
waste, because it means that everything else must constantly take locks for
no reason.

Maybe in the spirit of "you don't pay for what you don't use" the way
Python binding does it is actually fundamentally better, with an
EventInjector thingie that you can add to your Container if you want and
then call injector.inject(callback) instead of container.inject?

On Fri, Mar 24, 2017 at 3:30 PM, Alan Conway <ac...@redhat.com> wrote:

> On Fri, 2017-03-24 at 14:06 +0530, Venkat B wrote:
> > Good day,
> >
> > ---------- Forwarded message ----------
> > > From: Alan Conway <ac...@redhat.com>
> > > To: users@qpid.apache.org
> > > Cc:
> > > Bcc:
> > > Date: Wed, 22 Mar 2017 14:09:00 -0400
> > > Subject: Re: [qpid-proton-cpp] default_container does not implement
> > > thread
> > > safety at all?
> > >
> >
> > [snip]
> > >
> > > And to be clear this is abuse to hack out of a short-term hole - we
> > > will have a properly behaved solution soon.
> > >
> > >
> >
> > Do you have something that I could test?
> > The code in examples/cpp/mt does not compile out of the box on 0.17.0
> > and
> > in case it is being redesigned anyway then I would prefer to work
> > with the
> > new API.
> >
>
> I defer to astitcher on the current state of the examples. The API
> isn't changing significantly, the code in mt/broker.cpp will still be
> correct with only minor changes if any.
>
> The code in epoll_container.cpp will also still be basically unchanged
> but writing a custom container will no longer be necessary for most
> cases: the default_container will be thread-safe and we will have
> epoll-native, iocp-native and libuv flavours to cover most needs.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> For additional commands, e-mail: users-help@qpid.apache.org
>
>

Re: [qpid-proton-cpp] default_container does not implement thread safety at all?

[Alan Conway <ac...@redhat.com>]

On Fri, 2017-05-26 at 03:06 +0300, Fj wrote:
> By the way, if anyone is interested, as a stop-gap solution I ended
> up just
> implementing a timer-based callback reenqueuing itself every 50ms and
> pulling callbacks from a properly synchronized queue and executing
> them on
> the worker thread. It's not even that bad performance-wise, it just
> adds
> those 50/2 ms of latency on average.

That's the right workaround. The new C++ API has thread-safe work-
queues that will allow you to have a function executed in the proper
context which will be more efficient and more flexible than using the
timer callbacks. Behind the scenes it uses whatever is efficient on the
underlying IO platform (linux eventfds, libuv async_send etc.) to do
the wakeup.
 
> 
> A better solution would be to do the same thing the Python binding
> does,
> expose a Selectable interface and use it to wake up the reactor
> thread.
> 
> And by the way there's a thing that you might want to consider:
> there's a
> bunch of use cases that don't need inter-thread synchronization, for
> example, single-threaded workers, or multi-threaded workers that only
> need
> to know when to stop and they know it from the socket being closed.
> In
> those cases exposing a thread-safe Container.inject(callback) is a
> total
> waste, because it means that everything else must constantly take
> locks for
> no reason.

+1, those are all good observations. Look at the C proactors to see how
we are implementing this. Suggestions welcome. At the C level all you
can do is trigger a wakeup, to keep the C layer easy to re-implement.
The C++ binding will use the basic wakeup provided by C, and add
programmer convenience in the form of work queues so you can not only
trigger a wakeup but provide a function to be executed.

> 
> Maybe in the spirit of "you don't pay for what you don't use" the way
> Python binding does it is actually fundamentally better, with an
> EventInjector thingie that you can add to your Container if you want
> and
> then call injector.inject(callback) instead of container.inject?

That's roughly how it works: there are a set of "contexts", the
container is just one of them. Like selectables but simpler, providing
only "wakeup" not read/write. Each connection, listener and the
container have a context, we're thinking about how to expose the
concept so the user can create contexts of their own for syncing
application data structures.

> 
> On Fri, Mar 24, 2017 at 3:30 PM, Alan Conway <ac...@redhat.com>
> wrote:
> 
> > On Fri, 2017-03-24 at 14:06 +0530, Venkat B wrote:
> > > Good day,
> > > 
> > > ---------- Forwarded message ----------
> > > > From: Alan Conway <ac...@redhat.com>
> > > > To: users@qpid.apache.org
> > > > Cc:
> > > > Bcc:
> > > > Date: Wed, 22 Mar 2017 14:09:00 -0400
> > > > Subject: Re: [qpid-proton-cpp] default_container does not
> > > > implement
> > > > thread
> > > > safety at all?
> > > > 
> > > 
> > > [snip]
> > > > 
> > > > And to be clear this is abuse to hack out of a short-term hole
> > > > - we
> > > > will have a properly behaved solution soon.
> > > > 
> > > > 
> > > 
> > > Do you have something that I could test?
> > > The code in examples/cpp/mt does not compile out of the box on
> > > 0.17.0
> > > and
> > > in case it is being redesigned anyway then I would prefer to work
> > > with the
> > > new API.
> > > 
> > 
> > I defer to astitcher on the current state of the examples. The API
> > isn't changing significantly, the code in mt/broker.cpp will still
> > be
> > correct with only minor changes if any.
> > 
> > The code in epoll_container.cpp will also still be basically
> > unchanged
> > but writing a custom container will no longer be necessary for most
> > cases: the default_container will be thread-safe and we will have
> > epoll-native, iocp-native and libuv flavours to cover most needs.
> > 
> > 
> > 
> > -----------------------------------------------------------------
> > ----
> > To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> > For additional commands, e-mail: users-help@qpid.apache.org
> > 
> > 


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org