My 2 cents on the naming issue: I'm not convinced that a single queue is
the best metaphor for the Transport, even if qualified by the term
"transforming".  The meaning of the input and output data is surely so
different that calling it a queue masks the essence of what the engine
does.

To me, a transforming queue suggests something that spits out something
semantically identical to its input.  For example, a byte queue whose head
is a UTF-8-encoded transformation of its UTF-8 tail.  I don't think
Transport falls into this category, therefore my preference would be for
the words input and output to appear in the function names.

Phil


On 7 February 2013 14:23, Ken Giusti <kgiu...@redhat.com> wrote:

> "What we've got here is failure to communicate."
>
>
> > There aren't necessarily distinct
> > input/output
> > buffer objects, rather the whole transport interface itself is really
> > just
> > single structure (a [transforming] byte queue) with push/peek/pop
> > corresponding exactly to any standard queue interface.
>
> Aha!  Well, that explains it - I've always though that the transport was
> composed of two separate buffers - one for input, the other for output.  At
> least, that's my interpretation of the existing API.
>
> A "transforming byte queue" didn't immediately pop into my mind when
> reading these new APIs.  You may want to add a bit of documentation to that
> patch explaining this meme before the APIs are described.  Would be quite
> useful to anyone attempting to implement a driver.
>
>
> -K
>
> ----- Original Message -----
> > Looks like the attachement didn't make it. Here's the link to the
> > patch on
> > JIRA:
> >
> https://issues.apache.org/jira/secure/attachment/12568408/transport.patch
> >
> > --Rafael
> >
> > On Thu, Feb 7, 2013 at 8:10 AM, Rafael Schloming <r...@alum.mit.edu>
> > wrote:
> >
> > > Here's a patch to engine.h with updated docs/naming. I've
> > > documented what
> > > I believe would be future lifecycle semantics, however as I said
> > > before I
> > > think an initial patch would need to be more conservative. I think
> > > these
> > > names would also work with input/output prefixes, although as the
> > > interface
> > > now pretty much exactly matches a circular buffer (i.e. a byte
> > > queue), I
> > > find the input/output prefixes a bit jarring.
> > >
> > > --Rafael
> > >
> > >
> > > On Thu, Feb 7, 2013 at 5:53 AM, Rafael Schloming <r...@alum.mit.edu>
> > > wrote:
> > >
> > >> On Wed, Feb 6, 2013 at 5:12 PM, Rafael Schloming
> > >> <r...@alum.mit.edu>wrote:
> > >>
> > >>> On Wed, Feb 6, 2013 at 2:13 PM, Ken Giusti <kgiu...@redhat.com>
> > >>> wrote:
> > >>>
> > >>>> Rafi, I agree with the rational behind these changes.
> > >>>>
> > >>>> >     /** Return a pointer to a transport's input buffer. This
> > >>>> >     pointer
> > >>>> >     may
> > >>>> >      * change when calls into the engine are made.
> > >>>>
> > >>>> I think we'll need to be a little more liberal with the
> > >>>> lifecycle
> > >>>> guarantee of these buffer pointers.  Drivers based on
> > >>>> "completion" models
> > >>>> (rather than sockets) could be forced to do data copies rather
> > >>>> than
> > >>>> supplying the pointer directly to the completion mechanism.
> > >>>>
> > >>>
> > >>> That sentence was actually supposed to be deleted. The sentences
> > >>> after
> > >>> that describes the intended lifecycle policy for the input
> > >>> buffer:
> > >>>
> > >>>   Calls to ::pn_transport_push may change the value of this
> > >>>   pointer and
> > >>> the amount of free space reported by ::pn_transport_capacity.
> > >>>
> > >>>
> > >>>>
> > >>>> Could we instead guarantee that pointers (and space) returned
> > >>>> from the
> > >>>> transport remain valid until 1) the transport is released or 2)
> > >>>> the
> > >>>> "push"/"pop" call is made against the transport?
> > >>>>
> > >>>
> > >>> That is in fact what I intended for push. For pop this would
> > >>> place a lot
> > >>> more restrictions on the engine implementation. Say for example
> > >>> peek is
> > >>> called and then the top half is used to write message data.
> > >>> Ideally there
> > >>> should actually be more data to write over the network, which
> > >>> means that
> > >>> the transport may want to grow (realloc) the output buffer, and
> > >>> this of
> > >>> course is more complex if the external pointer needs to stay
> > >>> valid. Given
> > >>> that at worst this will incur an extra copy that is equivalent to
> > >>> what
> > >>> we're currently doing, I figured it would be safer to start out
> > >>> with more
> > >>> conservative semantics. We can always relax them later when we
> > >>> have had
> > >>> more time to consider the implementation.
> > >>>
> > >>>
> > >>>> Or perhaps a reference count-based interface would be safer?
> > >>>>  Once the
> > >>>> driver determines there is capacity/pending, it "reserves" the
> > >>>> buffer and
> > >>>> is required to "push"/"pop" to release it?
> > >>>>
> > >>>> Oh, and those names absolutely stink. ;)  It's unclear from the
> > >>>> function names what components of the transport they are
> > >>>> affecting.  I'd
> > >>>> rather something more readable:
> > >>>>
> > >>>> pn_transport_capacity() --> pn_transport_input_capacity()
> > >>>> pn_transport_buffer() -> pn_transport_input_buffer()
> > >>>> pn_transport_push() --> pn_transport_input_written()
> > >>>>
> > >>>
> > >>> I think your names (and my documentation) actually suffer from
> > >>> exposing
> > >>> too much of the implementation. There aren't necessarily distinct
> > >>> input/output buffer objects, rather the whole transport interface
> > >>> itself is
> > >>> really just single structure (a [transforming] byte queue) with
> > >>> push/peek/pop corresponding exactly to any standard queue
> > >>> interface.
> > >>>
> > >>> FWIW, I do agree pn_transport_buffer() could be better, however
> > >>> capacity, pending, push, peek, and pop all correspond to pretty
> > >>> much every
> > >>> standard queue/circular buffer/stack interface I've ever seen,
> > >>> and my hasty
> > >>> documentation notwithstanding, I think that is a good meme to
> > >>> take
> > >>> advantage of, particularly since the push/peek/pop semantics and
> > >>> their
> > >>> relationship to each other is actually quite important in
> > >>> guaranteeing that
> > >>> the pitfall occurring in PROTON-222 doesn't come up when people
> > >>> code their
> > >>> own drivers.
> > >>>
> > >>
> > >> To elaborate on this a little, I think the key part of the meme
> > >> that I
> > >> like is how peek strongly signals "non-destructive-read" and pop
> > >> strongly
> > >> signals a destructive operation. I don't get that same destructive
> > >> operation sense from output_written, and I think particularly when
> > >> considering the lifecycle semantics, the
> > >> non-destructive/destructive nature
> > >> of the operations is kind of key.
> > >>
> > >> FWIW, I can buy the input/output prefix as it's possibly equally
> > >> valid to
> > >> view the interface as a queue head and a queue tail that aren't on
> > >> the same
> > >> queue, however I would personally omit them for two reasons (1)
> > >> the notion
> > >> of a transforming queue signals to the user that when you provide
> > >> input,
> > >> you should expect there to be output, and (2) I appreciate
> > >> brevity. ;-)
> > >>
> > >> --Rafael
> > >>
> > >>
> > >
> >
>

Reply via email to