[java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

I'm starting to look at improving this areas as I was told a few folks have
noted some concerns.

I would appreciate some input on these concerns and hope to have a
discussion to figure out how best to proceed.

Regards,

Rajith

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Rafi,

Could you please explaining your approach to using the AbstractEncoder with
respect to starting and ending a frame.
Looking at the code it wasn't immediately clear to me on how you intended
to use this.

A quick code example will be quite useful.

Regards,

Rajith

On Thu, Jan 15, 2015 at 3:15 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> I don't believe there is an existing one.
>
> --Rafael
>
> On Thu, Jan 15, 2015 at 3:10 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
> > Rafi,
> >
> > Do you have a JIRA for this work?
> >
> > Regards,
> >
> > Rajith
> >
> > On Wed, Jan 14, 2015 at 6:35 PM, Rajith Muditha Attapattu <
> > rajith77@gmail.com> wrote:
> >
> > > Rafi,
> > >
> > > I had a closer look at the code, put it on trunk and ran your
> benchmark.
> > > I see quite an improvement with respect to writing lists, maps and
> > strings.
> > >
> > > Simply put the writeList and writeMap methods in the old encorder is
> > about
> > > ~10 times slower than the new encorder.
> > > If I run with a sufficiently large set of strings, the old encorder is
> > > about ~2 times slower than the new encorder.
> > >
> > > I'm now focusing on hooking it up with the engine.
> > >
> > > Once that is done we can look at tweaking it further. But as it is, the
> > > new codec is a real improvement over the existing one.
> > > Great job Rafi!
> > >
> > > Rajith
> > >
> > > On Tue, Jan 6, 2015 at 10:28 AM, Rajith Muditha Attapattu <
> > > rajith77@gmail.com> wrote:
> > >
> > >> Thanks Rafi, for the link.
> > >> I agree that any work should use this as a basis.
> > >>
> > >> I plan to take a closer look at this in the next week or so.
> > >>
> > >> Rajith
> > >>
> > >> On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <rh...@alum.mit.edu>
> > >> wrote:
> > >>
> > >>> A while back I implemented a relatively complete standalone codec
> here:
> > >>>
> > >>>
> > >>>
> > >>>
> >
> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
> > >>>
> > >>> It's quite a bit faster than the existing codec. I believe any new
> > codec
> > >>> work should probably be based on this. It's relatively standalone, so
> > >>> should be easy to import into the tree, and then it's just a matter
> of
> > >>> modifying the rest of the engine to use it. Note that my
> > qpid-proton-old
> > >>> repo is a clone of the pre-migration repo.
> > >>>
> > >>> --Rafael
> > >>>
> > >>>
> > >>> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
> > >>> rajith77@gmail.com> wrote:
> > >>> >
> > >>> > I'm starting to look at improving this areas as I was told a few
> > folks
> > >>> have
> > >>> > noted some concerns.
> > >>> >
> > >>> > I would appreciate some input on these concerns and hope to have a
> > >>> > discussion to figure out how best to proceed.
> > >>> >
> > >>> > Regards,
> > >>> >
> > >>> > Rajith
> > >>> >
> > >>>
> > >>
> > >>
> > >
> >
>

Re: [java] Message codec improvements

[Robbie Gemmell <ro...@gmail.com>]

On 20 February 2015 at 03:57, Rajith Muditha Attapattu
<ra...@gmail.com> wrote:
>> Most of the things I'd consider most awkward about about the current
>> codebase have little to do with the Message, but rather all the stuff
>> you need before you get that far.
>>
> Changing or improving Message or any of the public API's is not the focus
> for me.
> But while working on the type handling and codec stuff, I was also looking
> closely at how some of these "types" are used throughout the codebase and
> message handling was one such area.
> Given that I found the set method in the Message interface a bit of a
> nuisance to use, and I had to use it fairly often, I took the opportunity
> to start a discussion to see if an improvement could be made.
>

Which is entirely fair enough :)

A new mail thread might have been a good idea though, given its not
codec and is a breaking change to almost everyone.

>
>> Mostly, I find other areas being much more awkard, like many of the
>> types required to set up and operate connections, sessions, links etc
>> being in a bunch of different packages that arent as obvious as they
>> could be, and perhaps even interfaces and implementations with the
>> same name (e.g Source).
>
>
> Yep this has been noted and mentioned by several folks, and an attempt has
> been made to improve this area.
> I've got quite a large chuck on work sitting on my local that I will post
> to my branch soon.
>
>
>> I might have added a body specific interface
>> for set/getBody rather than just using the Section interface, since
>> that permits the other non-body sections.
>>
>>
> +1

It might even be worthwhile doing for 0.9. If im thinking it through
properly, no existing code that produces legal amqp messages would be
affected by such a change, since all 'Body' objects would continue to
be a Section, only the Body type Section objects should ever have been
used with the method, and all the other message sections have explicit
setters.

>
>>
>> > If I want to send a String or a Map, I have to wrap it with an AMQP type
>> > first. Which is awkward at best.
>> >
>>
>> As I said, I can definitely see there also being merit in having an
>> API that you set/get simple 'body objects' such as String with. I just
>> dont think it should be the only way as outlined, particularly since
>> it hasnt been from the start.
>>
>> I think setBody(Object) and Object getBody() is less restrictive than the
> current approach.
> It will certainly allow a user to pass an Object, Data or an AmqpSequence.
>

Its certainly less restrictive as a method signature, and could then
do almost anything as a result yes. It would obviously need better
documented to explain what it did do, which would become less obvious
in some ways and more intuitive in others.

It wasnt clear to me from your first mail that you atually intended to
keep it supporting setting the section types, but I mentioned that
possibility in my initial reply. Whilst I do still see some issues
with the setter the main things for me are probably around the getter
since as mentioned, an "Object getBody()" obviously cant permit the
existing behaviour and the new behaviour without e.g. a toggle to
control which behaviour it gives.

> The impl could simply return an Object if it's an amqp-value and Data or
> AmqpSequence if it's data or sequence.
> Even with the current approach you still need to use the instanceof
> operator to figure out the "type". So no difference there but slightly more
> user friendly.
>

Mixing the two behaviours in the same method as the same time doesnt
seem ideal to me. I'd probably prefer the toggle to do it all one way
or the other, or just separate methods that did one thing well would
probably be clearest for me.

> But the important thing to note here is that the current approach simply
> "restricts" to an argument of type Section, which I think is less user
> friendly.
>
> We don't need to remove the existing get/set methods. I have no problem
> leaving them there and it will save some compile errors.
> But if we add get/set Object it will certainly make them redundant. Just
> saying :)
>
> Would you agree?
>

I dont think we can add a second getObject method as our code wouldnt
compile, never mind the user code :)

I'll admit I wasnt thinking the change through fully either when
commenting in earlier mails, as even just changing to "Object getBody"
would obviously break compilation in many (but annoyingly, far from
all) cases for user code.

>>
>> >
>> >> Doing this would simply some use cases, but also further restrict the
>> >> ability to send the bodies you want or know what you really received,
>> >> e.g is a List an AmqpvValue+List or an AmqpSequence?
>> >
>> >
>> > From a typical application programmers perspective,  a List wrapped in an
>> > AmqpValue or an AmqpSequence makes little difference.
>>
>> It makes a fairly big difference if you e.g want to send sequences.
>>
>
> Looking at the spec. You can send only a single AmqpValue section but one
> or more AmqpSequence or Data sections.
> Unfortunately the current impl doesn't allow more than one sequence or data
> section to be sent which kills that advantage.
> We should implement that.
>

Fixing things for multiple bodies is also something I would like to
see and will need at some point. The setter would be easy, various
options for that, such as making it a vararg which that would keep
working for existing code as well. The getter would need thought,
possibly some sort of 'next body' increment or just a new method to
get them all in a list.


> I don't think I advocated anywhere about limiting what can be sent via the
> Message abstraction.
>
> I guess by getting into a discussion about why a user would care whether a
> single list is sent as a sequence or a value object, I gave the wrong
> impression that we should somehow not allow sending it as a sequence.
> I guess thats where you got the impression that I'm trying to limit what
> can or cannot be sent.
> Apologies for causing any confusion there.
>

In large part, yes. The other part is around what you would do to
return multiple bodies via the single "Object getBody" method, as
automagically combining them isnt necessarily something that I might
want as the receiver, or always as simple as it might sound.

> My point was that for most cases an average user would not care about the
> underlying AMQP type and wouldn't care about whether a single list or a
> byte[] is sent as an amqp-value vs sequence/data.
>
> For those that care they can simply wrap it with the AmqpSequence or Data.
>

As I have said from the start, I do see value in having such
simplicity available for use where it is appropriate. Just so long as
we arent artificially restricting everything to be done that way, and
personally I dont think we need to break existing code to add such
functionality.

>
>> > What the user cares about is that it's a List, not the underlying AMQP
>> type.
>> > Unless I have missed some subtlety, I don't see much of a benefit in
>> > figuring out what the underlying AMQP type is.
>> > If they care about that level of detail, then perhaps they are working at
>> > the wrong level of granularity.
>> > They should instead use the codec API for that.
>> >
>> > Robbie, can you please give us a use case to further explain your point?
>> >
>>
>> I'm mainly getting at that by making the proposed change, we would be
>> saying that the Message API probably wont actually support sending
>> lots of varieties of AMQP message content, or perhaps not receiving
>> them. Sequences allow you to do things that value+list do not, and
>> data allows you to do things that value+binary do not (I can only
>> presume thats part of the reason they both exist?).  The JMS mapping
>> for example uses sequences and data to allow for things that wouldnt
>> be possible using value+list or value+binary. Even if it didnt use
>> them as a matter of course for its basic message types though, it is
>> also a goal to let people send/recv fairly arbitrary AMQP message
>> through the JMS API, which could involve wanting to use them.
>>
>> Again, I'm all for adding a simpler way to do things which people can
>> use where those are sufficient, but it seems weird that it would be
>> made the only way, which would make Message be unable to do reasonable
>> things it can currently do, and will break most existing code over
>> something that isnt particularly hard to use as is, particularly when
>> compared to the rest of the codebase.
>>
>
> I'm fine about not removing the existing methods.
> But want to highlight again that what I proposed isn't restrictive compared
> to what we have today.
> Also what I proposed would make the existing methods redundant.
>
> Removing them will break existing code, but would also give an opportunity
> to simplify some of the code out there ;)
> Many API's do change over the years.
> But I don't care one way or the other. We could leave them as it is.
>
>
>> It also feels a little weird that something [originally] written
>> mostly by the key authors of the protocol, as a toolkit allowing
>> others to leverage the protocol without digging deep into a protocol
>> implementation, wouldnt actually let you use certain basic
>> functionality of the messages without doing so. Perhaps such people
>> should go and use the codec, but will it become similarly hobbled at
>> some point?
>>
>
> You raise a good point. Best answered by the folks who wrote the code, as I
> can only speculate at best ;).
>
>
>>
>> >
>> >> Is <some-object>
>> >> an AmqpValue+Binary, or is it a Data?
>> >
>> >
>> > Again the same question. For 99% of the use cases out there, the user
>> will
>> > only care about the underlying byte[].
>> > Could you pls let us know a use case where such a distinction is useful ?
>> >
>> >
>> >> There is then things like how to
>> >>
>> > support e.g multiple Data and AmqpSequence sections (something it
>> >> should already support but doesnt).
>> >>
>> >> Yea I noticed this was missing. More thoughts on that later.
>> >
>> >
>> >> Robbie
>> >>
>>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

> Most of the things I'd consider most awkward about about the current
> codebase have little to do with the Message, but rather all the stuff
> you need before you get that far.
>
> Changing or improving Message or any of the public API's is not the focus
for me.
But while working on the type handling and codec stuff, I was also looking
closely at how some of these "types" are used throughout the codebase and
message handling was one such area.
Given that I found the set method in the Message interface a bit of a
nuisance to use, and I had to use it fairly often, I took the opportunity
to start a discussion to see if an improvement could be made.


> Mostly, I find other areas being much more awkard, like many of the
> types required to set up and operate connections, sessions, links etc
> being in a bunch of different packages that arent as obvious as they
> could be, and perhaps even interfaces and implementations with the
> same name (e.g Source).


Yep this has been noted and mentioned by several folks, and an attempt has
been made to improve this area.
I've got quite a large chuck on work sitting on my local that I will post
to my branch soon.


> I might have added a body specific interface
> for set/getBody rather than just using the Section interface, since
> that permits the other non-body sections.
>
>
+1

>
> > If I want to send a String or a Map, I have to wrap it with an AMQP type
> > first. Which is awkward at best.
> >
>
> As I said, I can definitely see there also being merit in having an
> API that you set/get simple 'body objects' such as String with. I just
> dont think it should be the only way as outlined, particularly since
> it hasnt been from the start.
>
> I think setBody(Object) and Object getBody() is less restrictive than the
current approach.
It will certainly allow a user to pass an Object, Data or an AmqpSequence.

The impl could simply return an Object if it's an amqp-value and Data or
AmqpSequence if it's data or sequence.
Even with the current approach you still need to use the instanceof
operator to figure out the "type". So no difference there but slightly more
user friendly.

But the important thing to note here is that the current approach simply
"restricts" to an argument of type Section, which I think is less user
friendly.

We don't need to remove the existing get/set methods. I have no problem
leaving them there and it will save some compile errors.
But if we add get/set Object it will certainly make them redundant. Just
saying :)

Would you agree?

>
> >
> >> Doing this would simply some use cases, but also further restrict the
> >> ability to send the bodies you want or know what you really received,
> >> e.g is a List an AmqpvValue+List or an AmqpSequence?
> >
> >
> > From a typical application programmers perspective,  a List wrapped in an
> > AmqpValue or an AmqpSequence makes little difference.
>
> It makes a fairly big difference if you e.g want to send sequences.
>

Looking at the spec. You can send only a single AmqpValue section but one
or more AmqpSequence or Data sections.
Unfortunately the current impl doesn't allow more than one sequence or data
section to be sent which kills that advantage.
We should implement that.

I don't think I advocated anywhere about limiting what can be sent via the
Message abstraction.

I guess by getting into a discussion about why a user would care whether a
single list is sent as a sequence or a value object, I gave the wrong
impression that we should somehow not allow sending it as a sequence.
I guess thats where you got the impression that I'm trying to limit what
can or cannot be sent.
Apologies for causing any confusion there.

My point was that for most cases an average user would not care about the
underlying AMQP type and wouldn't care about whether a single list or a
byte[] is sent as an amqp-value vs sequence/data.

For those that care they can simply wrap it with the AmqpSequence or Data.


> > What the user cares about is that it's a List, not the underlying AMQP
> type.
> > Unless I have missed some subtlety, I don't see much of a benefit in
> > figuring out what the underlying AMQP type is.
> > If they care about that level of detail, then perhaps they are working at
> > the wrong level of granularity.
> > They should instead use the codec API for that.
> >
> > Robbie, can you please give us a use case to further explain your point?
> >
>
> I'm mainly getting at that by making the proposed change, we would be
> saying that the Message API probably wont actually support sending
> lots of varieties of AMQP message content, or perhaps not receiving
> them. Sequences allow you to do things that value+list do not, and
> data allows you to do things that value+binary do not (I can only
> presume thats part of the reason they both exist?).  The JMS mapping
> for example uses sequences and data to allow for things that wouldnt
> be possible using value+list or value+binary. Even if it didnt use
> them as a matter of course for its basic message types though, it is
> also a goal to let people send/recv fairly arbitrary AMQP message
> through the JMS API, which could involve wanting to use them.
>
> Again, I'm all for adding a simpler way to do things which people can
> use where those are sufficient, but it seems weird that it would be
> made the only way, which would make Message be unable to do reasonable
> things it can currently do, and will break most existing code over
> something that isnt particularly hard to use as is, particularly when
> compared to the rest of the codebase.
>

I'm fine about not removing the existing methods.
But want to highlight again that what I proposed isn't restrictive compared
to what we have today.
Also what I proposed would make the existing methods redundant.

Removing them will break existing code, but would also give an opportunity
to simplify some of the code out there ;)
Many API's do change over the years.
But I don't care one way or the other. We could leave them as it is.


> It also feels a little weird that something [originally] written
> mostly by the key authors of the protocol, as a toolkit allowing
> others to leverage the protocol without digging deep into a protocol
> implementation, wouldnt actually let you use certain basic
> functionality of the messages without doing so. Perhaps such people
> should go and use the codec, but will it become similarly hobbled at
> some point?
>

You raise a good point. Best answered by the folks who wrote the code, as I
can only speculate at best ;).


>
> >
> >> Is <some-object>
> >> an AmqpValue+Binary, or is it a Data?
> >
> >
> > Again the same question. For 99% of the use cases out there, the user
> will
> > only care about the underlying byte[].
> > Could you pls let us know a use case where such a distinction is useful ?
> >
> >
> >> There is then things like how to
> >>
> > support e.g multiple Data and AmqpSequence sections (something it
> >> should already support but doesnt).
> >>
> >> Yea I noticed this was missing. More thoughts on that later.
> >
> >
> >> Robbie
> >>
>

Re: [java] Message codec improvements

[Robbie Gemmell <ro...@gmail.com>]

On 19 February 2015 at 16:11, Rajith Muditha Attapattu
<ra...@gmail.com> wrote:
> On Thu, Feb 19, 2015 at 5:41 AM, Robbie Gemmell <ro...@gmail.com>
> wrote:
>
>> On 19 February 2015 at 04:22, Rafael Schloming <rh...@alum.mit.edu> wrote:
>> > On Wed, Feb 18, 2015 at 3:58 PM, Rajith Muditha Attapattu <
>> > rajith77@gmail.com> wrote:
>> >
>> >> Setting the message body for an o.a.q.proton.message.Message is slightly
>> >> awkward.
>> >> You have to create a AmqpValue. AmqpSequence or a Data object that
>> >> encapsulates the underlying data type.
>> >>
>> >> Given that one of our goals is to expose an API that works with standard
>> >> Java types instead of AMQP defined types, I suggest we simply use Object
>> >> for setBody and getBody methods.
>> >> The implementation can then handle the conversion back and forth
>> >> underneath.
>> >>
>> >> What do you think?
>> >>
>> >
>> > Makes sense to me.
>> >
>> > --Rafael
>>
>> Adding additional functionality to set/get a 'body object' like this
>> certainly might be useful in cases, but I dont think it should be the
>> only way. I also wouldnt change the existing methods to do this unless
>> the idea is to let you continue setting/getting the existing Section
>> objects unchanged (it would obviously likely break all existing user
>> code sending payloads otherwise). I can see that being easy enough for
>> the setter as it works now, though I dont see how it could work for
>> the getter without adding a control toggle.
>>
>
> The Message interface is part of the public API and I understand the impact
> on existing code out there.
> **I don't plan to remove anything or even change any existing public API.
> At least not for the upcoming release.**

Yes, I understand that this isnt aimed at 0.9. When it lands doesnt
particularly change my perspective however.

>
> So far I have sketched out a parallel API for certain pieces of the public
> API. Message handling is one of them.
> What I would like to have is a discussion based on that, as it appears the
> existing API had little discussion or scrutiny.
>
> An API should be easy to use and work with standard Java types as much as
> possible. The existing API is awkward to use in that respect.

I'm not saying it has been scrutinized, but in regards to the setBody
specifically, I dont think it was chosen particularly randomly. There
are certainly other AMQP 1.0 API's that exist which work quite
similarly in that particular regard.

Easy to use doesn't necessarily mean using the Java types. Where it
makes sense, sure. Where they can't fully support the basic
functionality, using additional types seems fine and there are
obviously many which do that.

Most of the things I'd consider most awkward about about the current
codebase have little to do with the Message, but rather all the stuff
you need before you get that far. The main thing that would seem
confusing about Message to me are bits like the encode/decode vs
load/save stuff, and all the related methods for the latter that dont
really say what they do, but involve gems like the 'setMessageFormat'
method that doesnt set the 'Message Format' field of the AMQP message
transfer but rather has some other unspecified behaviour you need to
read the code to figure out.

Mostly, I find other areas being much more awkard, like many of the
types required to set up and operate connections, sessions, links etc
being in a bunch of different packages that arent as obvious as they
could be, and perhaps even interfaces and implementations with the
same name (e.g Source). That is all possibly in part because there was
only impl initially, then it got split to completely separate
API+Impl, then it all got pushed back together again. There were
efforts at having a bunch of separate factory objects, then the
aggregating Proton factory object, then yet-different seperate factory
objects, and also new factory objects in the interfaces. Figuring out
those types of things are far more annoying to get your head around in
my view. If you get as far as needing to set a body section, you have
done much harder work. I might have added a body specific interface
for set/getBody rather than just using the Section interface, since
that permits the other non-body sections.

> If I want to send a String or a Map, I have to wrap it with an AMQP type
> first. Which is awkward at best.
>

As I said, I can definitely see there also being merit in having an
API that you set/get simple 'body objects' such as String with. I just
dont think it should be the only way as outlined, particularly since
it hasnt been from the start.

>
>
>> Doing this would simply some use cases, but also further restrict the
>> ability to send the bodies you want or know what you really received,
>> e.g is a List an AmqpvValue+List or an AmqpSequence?
>
>
> From a typical application programmers perspective,  a List wrapped in an
> AmqpValue or an AmqpSequence makes little difference.

It makes a fairly big difference if you e.g want to send sequences.

> What the user cares about is that it's a List, not the underlying AMQP type.
> Unless I have missed some subtlety, I don't see much of a benefit in
> figuring out what the underlying AMQP type is.
> If they care about that level of detail, then perhaps they are working at
> the wrong level of granularity.
> They should instead use the codec API for that.
>
> Robbie, can you please give us a use case to further explain your point?
>

I'm mainly getting at that by making the proposed change, we would be
saying that the Message API probably wont actually support sending
lots of varieties of AMQP message content, or perhaps not receiving
them. Sequences allow you to do things that value+list do not, and
data allows you to do things that value+binary do not (I can only
presume thats part of the reason they both exist?).  The JMS mapping
for example uses sequences and data to allow for things that wouldnt
be possible using value+list or value+binary. Even if it didnt use
them as a matter of course for its basic message types though, it is
also a goal to let people send/recv fairly arbitrary AMQP message
through the JMS API, which could involve wanting to use them.

Again, I'm all for adding a simpler way to do things which people can
use where those are sufficient, but it seems weird that it would be
made the only way, which would make Message be unable to do reasonable
things it can currently do, and will break most existing code over
something that isnt particularly hard to use as is, particularly when
compared to the rest of the codebase.

It also feels a little weird that something [originally] written
mostly by the key authors of the protocol, as a toolkit allowing
others to leverage the protocol without digging deep into a protocol
implementation, wouldnt actually let you use certain basic
functionality of the messages without doing so. Perhaps such people
should go and use the codec, but will it become similarly hobbled at
some point?

>
>> Is <some-object>
>> an AmqpValue+Binary, or is it a Data?
>
>
> Again the same question. For 99% of the use cases out there, the user will
> only care about the underlying byte[].
> Could you pls let us know a use case where such a distinction is useful ?
>
>
>> There is then things like how to
>>
> support e.g multiple Data and AmqpSequence sections (something it
>> should already support but doesnt).
>>
>> Yea I noticed this was missing. More thoughts on that later.
>
>
>> Robbie
>>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

On Thu, Feb 19, 2015 at 5:41 AM, Robbie Gemmell <ro...@gmail.com>
wrote:

> On 19 February 2015 at 04:22, Rafael Schloming <rh...@alum.mit.edu> wrote:
> > On Wed, Feb 18, 2015 at 3:58 PM, Rajith Muditha Attapattu <
> > rajith77@gmail.com> wrote:
> >
> >> Setting the message body for an o.a.q.proton.message.Message is slightly
> >> awkward.
> >> You have to create a AmqpValue. AmqpSequence or a Data object that
> >> encapsulates the underlying data type.
> >>
> >> Given that one of our goals is to expose an API that works with standard
> >> Java types instead of AMQP defined types, I suggest we simply use Object
> >> for setBody and getBody methods.
> >> The implementation can then handle the conversion back and forth
> >> underneath.
> >>
> >> What do you think?
> >>
> >
> > Makes sense to me.
> >
> > --Rafael
>
> Adding additional functionality to set/get a 'body object' like this
> certainly might be useful in cases, but I dont think it should be the
> only way. I also wouldnt change the existing methods to do this unless
> the idea is to let you continue setting/getting the existing Section
> objects unchanged (it would obviously likely break all existing user
> code sending payloads otherwise). I can see that being easy enough for
> the setter as it works now, though I dont see how it could work for
> the getter without adding a control toggle.
>

The Message interface is part of the public API and I understand the impact
on existing code out there.
**I don't plan to remove anything or even change any existing public API.
At least not for the upcoming release.**

So far I have sketched out a parallel API for certain pieces of the public
API. Message handling is one of them.
What I would like to have is a discussion based on that, as it appears the
existing API had little discussion or scrutiny.

An API should be easy to use and work with standard Java types as much as
possible. The existing API is awkward to use in that respect.
If I want to send a String or a Map, I have to wrap it with an AMQP type
first. Which is awkward at best.



> Doing this would simply some use cases, but also further restrict the
> ability to send the bodies you want or know what you really received,
> e.g is a List an AmqpvValue+List or an AmqpSequence?


>From a typical application programmers perspective,  a List wrapped in an
AmqpValue or an AmqpSequence makes little difference.
What the user cares about is that it's a List, not the underlying AMQP type.
Unless I have missed some subtlety, I don't see much of a benefit in
figuring out what the underlying AMQP type is.
If they care about that level of detail, then perhaps they are working at
the wrong level of granularity.
They should instead use the codec API for that.

Robbie, can you please give us a use case to further explain your point?


> Is <some-object>
> an AmqpValue+Binary, or is it a Data?


Again the same question. For 99% of the use cases out there, the user will
only care about the underlying byte[].
Could you pls let us know a use case where such a distinction is useful ?


> There is then things like how to
>
support e.g multiple Data and AmqpSequence sections (something it
> should already support but doesnt).
>
> Yea I noticed this was missing. More thoughts on that later.


> Robbie
>

Re: [java] Message codec improvements

[Robbie Gemmell <ro...@gmail.com>]

On 19 February 2015 at 04:22, Rafael Schloming <rh...@alum.mit.edu> wrote:
> On Wed, Feb 18, 2015 at 3:58 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
>> Setting the message body for an o.a.q.proton.message.Message is slightly
>> awkward.
>> You have to create a AmqpValue. AmqpSequence or a Data object that
>> encapsulates the underlying data type.
>>
>> Given that one of our goals is to expose an API that works with standard
>> Java types instead of AMQP defined types, I suggest we simply use Object
>> for setBody and getBody methods.
>> The implementation can then handle the conversion back and forth
>> underneath.
>>
>> What do you think?
>>
>
> Makes sense to me.
>
> --Rafael

Adding additional functionality to set/get a 'body object' like this
certainly might be useful in cases, but I dont think it should be the
only way. I also wouldnt change the existing methods to do this unless
the idea is to let you continue setting/getting the existing Section
objects unchanged (it would obviously likely break all existing user
code sending payloads otherwise). I can see that being easy enough for
the setter as it works now, though I dont see how it could work for
the getter without adding a control toggle.

Doing this would simply some use cases, but also further restrict the
ability to send the bodies you want or know what you really received,
e.g is a List an AmqpvValue+List or an AmqpSequence? Is <some-object>
an AmqpValue+Binary, or is it a Data? There is then things like how to
support e.g multiple Data and AmqpSequence sections (something it
should already support but doesnt).

Robbie

Re: [java] Message codec improvements

[Rafael Schloming <rh...@alum.mit.edu>]

On Wed, Feb 18, 2015 at 3:58 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> Setting the message body for an o.a.q.proton.message.Message is slightly
> awkward.
> You have to create a AmqpValue. AmqpSequence or a Data object that
> encapsulates the underlying data type.
>
> Given that one of our goals is to expose an API that works with standard
> Java types instead of AMQP defined types, I suggest we simply use Object
> for setBody and getBody methods.
> The implementation can then handle the conversion back and forth
> underneath.
>
> What do you think?
>

Makes sense to me.

--Rafael

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Setting the message body for an o.a.q.proton.message.Message is slightly
awkward.
You have to create a AmqpValue. AmqpSequence or a Data object that
encapsulates the underlying data type.

Given that one of our goals is to expose an API that works with standard
Java types instead of AMQP defined types, I suggest we simply use Object
for setBody and getBody methods.
The implementation can then handle the conversion back and forth underneath.

What do you think?

On Mon, Feb 2, 2015 at 4:43 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> Overall I think this is a good direction. I made a bunch of more detailed
> comments on the patch.
>
> --Rafael
>
> On Mon, Feb 2, 2015 at 1:42 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
>> Rafi, further to our discussion I have posted a patch to illustrate the
>> approach we plan to take.
>> This should enable me to make progress until you get a chance to make
>> further changes on the Decoder side.
>>
>> Regards,
>>
>> Rajith
>>
>> On Thu, Jan 29, 2015 at 3:59 PM, Rajith Muditha Attapattu <
>> rajith77@gmail.com> wrote:
>>
>>> More questions.
>>>
>>> For all the maps we return should we restrict them to <String, Object>
>>> or should it be <Object, Object> ?
>>> Technically one could use a Number (int, long) etc as a key..
>>>
>>> Any opinion here? ;)
>>>
>>> On Thu, Jan 29, 2015 at 12:48 PM, Rafael Schloming <rh...@alum.mit.edu>
>>> wrote:
>>>
>>>> On Thu, Jan 29, 2015 at 12:28 PM, Rajith Muditha Attapattu <
>>>> rajith77@gmail.com> wrote:
>>>>
>>>>> Rafi could u pls answer the two questions I had in the code?
>>>>>
>>>>> 1. Your uint method only takes an int .. shouldn't it take a long?
>>>>> Bcos it could contain a value larger than a java int?
>>>>>
>>>>
>>>> To be honest I don't quite remember for sure, but I think it will do
>>>> the two's complement and put it on the wire as a proper unsigned value. In
>>>> other words I think it's just using the int type as a convenient/efficient
>>>> way to pass around 4 bytes.
>>>>
>>>>
>>>>>
>>>>> 2. What should I use for boolean? there is no getBoolean .. or an
>>>>> equivalent method
>>>>>
>>>>
>>>> I think you might need to add this. I probably just omitted it.
>>>>
>>>> --Rafael
>>>>
>>>
>>>
>>
>

Re: [java] Message codec improvements

[Rafael Schloming <rh...@alum.mit.edu>]

Overall I think this is a good direction. I made a bunch of more detailed
comments on the patch.

--Rafael

On Mon, Feb 2, 2015 at 1:42 PM, Rajith Muditha Attapattu <rajith77@gmail.com
> wrote:

> Rafi, further to our discussion I have posted a patch to illustrate the
> approach we plan to take.
> This should enable me to make progress until you get a chance to make
> further changes on the Decoder side.
>
> Regards,
>
> Rajith
>
> On Thu, Jan 29, 2015 at 3:59 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
>> More questions.
>>
>> For all the maps we return should we restrict them to <String, Object> or
>> should it be <Object, Object> ?
>> Technically one could use a Number (int, long) etc as a key..
>>
>> Any opinion here? ;)
>>
>> On Thu, Jan 29, 2015 at 12:48 PM, Rafael Schloming <rh...@alum.mit.edu>
>> wrote:
>>
>>> On Thu, Jan 29, 2015 at 12:28 PM, Rajith Muditha Attapattu <
>>> rajith77@gmail.com> wrote:
>>>
>>>> Rafi could u pls answer the two questions I had in the code?
>>>>
>>>> 1. Your uint method only takes an int .. shouldn't it take a long? Bcos
>>>> it could contain a value larger than a java int?
>>>>
>>>
>>> To be honest I don't quite remember for sure, but I think it will do the
>>> two's complement and put it on the wire as a proper unsigned value. In
>>> other words I think it's just using the int type as a convenient/efficient
>>> way to pass around 4 bytes.
>>>
>>>
>>>>
>>>> 2. What should I use for boolean? there is no getBoolean .. or an
>>>> equivalent method
>>>>
>>>
>>> I think you might need to add this. I probably just omitted it.
>>>
>>> --Rafael
>>>
>>
>>
>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Rafi, further to our discussion I have posted a patch to illustrate the
approach we plan to take.
This should enable me to make progress until you get a chance to make
further changes on the Decoder side.

Regards,

Rajith

On Thu, Jan 29, 2015 at 3:59 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> More questions.
>
> For all the maps we return should we restrict them to <String, Object> or
> should it be <Object, Object> ?
> Technically one could use a Number (int, long) etc as a key..
>
> Any opinion here? ;)
>
> On Thu, Jan 29, 2015 at 12:48 PM, Rafael Schloming <rh...@alum.mit.edu>
> wrote:
>
>> On Thu, Jan 29, 2015 at 12:28 PM, Rajith Muditha Attapattu <
>> rajith77@gmail.com> wrote:
>>
>>> Rafi could u pls answer the two questions I had in the code?
>>>
>>> 1. Your uint method only takes an int .. shouldn't it take a long? Bcos
>>> it could contain a value larger than a java int?
>>>
>>
>> To be honest I don't quite remember for sure, but I think it will do the
>> two's complement and put it on the wire as a proper unsigned value. In
>> other words I think it's just using the int type as a convenient/efficient
>> way to pass around 4 bytes.
>>
>>
>>>
>>> 2. What should I use for boolean? there is no getBoolean .. or an
>>> equivalent method
>>>
>>
>> I think you might need to add this. I probably just omitted it.
>>
>> --Rafael
>>
>
>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

More questions.

For all the maps we return should we restrict them to <String, Object> or
should it be <Object, Object> ?
Technically one could use a Number (int, long) etc as a key..

Any opinion here? ;)

On Thu, Jan 29, 2015 at 12:48 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> On Thu, Jan 29, 2015 at 12:28 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
>> Rafi could u pls answer the two questions I had in the code?
>>
>> 1. Your uint method only takes an int .. shouldn't it take a long? Bcos
>> it could contain a value larger than a java int?
>>
>
> To be honest I don't quite remember for sure, but I think it will do the
> two's complement and put it on the wire as a proper unsigned value. In
> other words I think it's just using the int type as a convenient/efficient
> way to pass around 4 bytes.
>
>
>>
>> 2. What should I use for boolean? there is no getBoolean .. or an
>> equivalent method
>>
>
> I think you might need to add this. I probably just omitted it.
>
> --Rafael
>

Re: [java] Message codec improvements

[Rafael Schloming <rh...@alum.mit.edu>]

On Thu, Jan 29, 2015 at 12:28 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> Rafi could u pls answer the two questions I had in the code?
>
> 1. Your uint method only takes an int .. shouldn't it take a long? Bcos it
> could contain a value larger than a java int?
>

To be honest I don't quite remember for sure, but I think it will do the
two's complement and put it on the wire as a proper unsigned value. In
other words I think it's just using the int type as a convenient/efficient
way to pass around 4 bytes.


>
> 2. What should I use for boolean? there is no getBoolean .. or an
> equivalent method
>

I think you might need to add this. I probably just omitted it.

--Rafael

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Rafi could u pls answer the two questions I had in the code?

1. Your uint method only takes an int .. shouldn't it take a long? Bcos it
could contain a value larger than a java int?

2. What should I use for boolean? there is no getBoolean .. or an
equivalent method

Regards,

Rajith

On Wed, Jan 28, 2015 at 3:06 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

>
> On Wed, Jan 28, 2015 at 2:23 PM, Rafael Schloming <rh...@alum.mit.edu>
> wrote:
>
>> On Wed, Jan 28, 2015 at 2:01 PM, Rajith Muditha Attapattu <
>> rajith77@gmail.com> wrote:
>>
>> > Rafi,
>> >
>> > I just checked in some skeleton code to explore a particular approach.
>> > It avoids the intermediate objects we have in proton now (Ex
>> FlowType.java)
>> > Instead the Flow class is directly used by the encoding/decoding layer.
>> >
>> > The Flow class uses java types as opposed to AMQP specific types, which
>> > will address one of your main concerns.
>> >
>> > The code is checked into the "rajith-codec" branch.
>> > The classes to look at
>> > Flow.java in the o/a/q/p/transport2  package
>> > TransportTypesEncoder and TransportTypesDecoder in o/a/q/p/codec2
>> package.
>> >
>> > I've also put some question marks in the code .. pls answer them.
>> > But more importantly pls provide feedback on the direction taken.
>> >
>> > I'm keen to settle down on the direction/approach first and then will
>> start
>> > implementing the rest.
>> >
>>
>> It looks like a good start from what I saw from the commit message that
>> flew through my inbox. One general question, are you intending to develop
>> the rest of the performatives by hand, or is this just a skeleton to
>> figure
>> out what your generated output should look like, or is that TBD?
>>
>
> At this point I'm undecided. I will write a few more Performatives by hand
> and then figure out if it can be automated.
> It's not a lot of classes either .. so writing them by hand might not be a
> huge issue.
>
>
>>
>> If you put it up on reviewboard or make a PR or something I can make some
>> more detailed line by line comments and answer the questions you embedded
>> in the code.
>>
>
> https://reviews.apache.org/r/30379/
>
> Thanks in advance for your comments/feedback.
>
>
>>
>> --Rafael
>>
>
>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

On Wed, Jan 28, 2015 at 2:23 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> On Wed, Jan 28, 2015 at 2:01 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
> > Rafi,
> >
> > I just checked in some skeleton code to explore a particular approach.
> > It avoids the intermediate objects we have in proton now (Ex
> FlowType.java)
> > Instead the Flow class is directly used by the encoding/decoding layer.
> >
> > The Flow class uses java types as opposed to AMQP specific types, which
> > will address one of your main concerns.
> >
> > The code is checked into the "rajith-codec" branch.
> > The classes to look at
> > Flow.java in the o/a/q/p/transport2  package
> > TransportTypesEncoder and TransportTypesDecoder in o/a/q/p/codec2
> package.
> >
> > I've also put some question marks in the code .. pls answer them.
> > But more importantly pls provide feedback on the direction taken.
> >
> > I'm keen to settle down on the direction/approach first and then will
> start
> > implementing the rest.
> >
>
> It looks like a good start from what I saw from the commit message that
> flew through my inbox. One general question, are you intending to develop
> the rest of the performatives by hand, or is this just a skeleton to figure
> out what your generated output should look like, or is that TBD?
>

At this point I'm undecided. I will write a few more Performatives by hand
and then figure out if it can be automated.
It's not a lot of classes either .. so writing them by hand might not be a
huge issue.


>
> If you put it up on reviewboard or make a PR or something I can make some
> more detailed line by line comments and answer the questions you embedded
> in the code.
>

https://reviews.apache.org/r/30379/

Thanks in advance for your comments/feedback.


>
> --Rafael
>

Re: [java] Message codec improvements

[Rafael Schloming <rh...@alum.mit.edu>]

On Wed, Jan 28, 2015 at 2:01 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> Rafi,
>
> I just checked in some skeleton code to explore a particular approach.
> It avoids the intermediate objects we have in proton now (Ex FlowType.java)
> Instead the Flow class is directly used by the encoding/decoding layer.
>
> The Flow class uses java types as opposed to AMQP specific types, which
> will address one of your main concerns.
>
> The code is checked into the "rajith-codec" branch.
> The classes to look at
> Flow.java in the o/a/q/p/transport2  package
> TransportTypesEncoder and TransportTypesDecoder in o/a/q/p/codec2 package.
>
> I've also put some question marks in the code .. pls answer them.
> But more importantly pls provide feedback on the direction taken.
>
> I'm keen to settle down on the direction/approach first and then will start
> implementing the rest.
>

It looks like a good start from what I saw from the commit message that
flew through my inbox. One general question, are you intending to develop
the rest of the performatives by hand, or is this just a skeleton to figure
out what your generated output should look like, or is that TBD?

If you put it up on reviewboard or make a PR or something I can make some
more detailed line by line comments and answer the questions you embedded
in the code.

--Rafael

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Rafi,

I just checked in some skeleton code to explore a particular approach.
It avoids the intermediate objects we have in proton now (Ex FlowType.java)
Instead the Flow class is directly used by the encoding/decoding layer.

The Flow class uses java types as opposed to AMQP specific types, which
will address one of your main concerns.

The code is checked into the "rajith-codec" branch.
The classes to look at
Flow.java in the o/a/q/p/transport2  package
TransportTypesEncoder and TransportTypesDecoder in o/a/q/p/codec2 package.

I've also put some question marks in the code .. pls answer them.
But more importantly pls provide feedback on the direction taken.

I'm keen to settle down on the direction/approach first and then will start
implementing the rest.

Regards,

Rajith


On Tue, Jan 27, 2015 at 8:13 AM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> My inclination here would be to keep things simple. One of the more
> annoying aspects of working on the Java code is dealing with the fact that
> all the AMQP performatives use an entirely different type system. I would
> start out with developing a plain and simple POJO representation for all
> the performatives defined by the protocol.
>
> By POJO, I mean for example a Flow class where getCredit() actually returns
> a standard java type (probably a long) rather than an
> org.apache.qpid.proton.amqp.UnsignedInteger which I then need to convert
> into a long in order to do anything useful with.
>
> You could use some code generation here, but I wouldn't sacrifice the
> naturalness of the API just to allow it to be code generated, and I
> wouldn't think of this as an internal API just for the engine to use. We
> should have something here that anyone could use to easily decode/encode
> raw AMQP performatives in a natural way.
>
> Once you have those classes, it should be straightforward to write a
> DataHandler that will use the fast codec to produce them.
>
> --Rafael
>
>
> On Fri, Jan 23, 2015 at 12:09 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
> > Given the existing encorder was not put behind any sort of interface (the
> > impl is directly used), it's proving to be a PIA to track down all the
> > different types and changing them to use the new codec.
> > Look at the "public static void register(Decoder decoder, EncoderImpl
> > encoder)" method of any of the XXXType classes to get a sense of what I'm
> > describing.
> >
> > We should take this opportunity to review the current approach and see if
> > we could come up with a better alternative.
> > If anybody has any suggestions, I would be happy to discuss them with you
> > on the list.
> >
> > Regards,
> >
> > Rajith
> >
> >
> >
> >
> > On Thu, Jan 15, 2015 at 3:15 PM, Rafael Schloming <rh...@alum.mit.edu>
> > wrote:
> >
> > > I don't believe there is an existing one.
> > >
> > > --Rafael
> > >
> > > On Thu, Jan 15, 2015 at 3:10 PM, Rajith Muditha Attapattu <
> > > rajith77@gmail.com> wrote:
> > >
> > > > Rafi,
> > > >
> > > > Do you have a JIRA for this work?
> > > >
> > > > Regards,
> > > >
> > > > Rajith
> > > >
> > > > On Wed, Jan 14, 2015 at 6:35 PM, Rajith Muditha Attapattu <
> > > > rajith77@gmail.com> wrote:
> > > >
> > > > > Rafi,
> > > > >
> > > > > I had a closer look at the code, put it on trunk and ran your
> > > benchmark.
> > > > > I see quite an improvement with respect to writing lists, maps and
> > > > strings.
> > > > >
> > > > > Simply put the writeList and writeMap methods in the old encorder
> is
> > > > about
> > > > > ~10 times slower than the new encorder.
> > > > > If I run with a sufficiently large set of strings, the old encorder
> > is
> > > > > about ~2 times slower than the new encorder.
> > > > >
> > > > > I'm now focusing on hooking it up with the engine.
> > > > >
> > > > > Once that is done we can look at tweaking it further. But as it is,
> > the
> > > > > new codec is a real improvement over the existing one.
> > > > > Great job Rafi!
> > > > >
> > > > > Rajith
> > > > >
> > > > > On Tue, Jan 6, 2015 at 10:28 AM, Rajith Muditha Attapattu <
> > > > > rajith77@gmail.com> wrote:
> > > > >
> > > > >> Thanks Rafi, for the link.
> > > > >> I agree that any work should use this as a basis.
> > > > >>
> > > > >> I plan to take a closer look at this in the next week or so.
> > > > >>
> > > > >> Rajith
> > > > >>
> > > > >> On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <
> rhs@alum.mit.edu
> > >
> > > > >> wrote:
> > > > >>
> > > > >>> A while back I implemented a relatively complete standalone codec
> > > here:
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>>
> > > >
> > >
> >
> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
> > > > >>>
> > > > >>> It's quite a bit faster than the existing codec. I believe any
> new
> > > > codec
> > > > >>> work should probably be based on this. It's relatively
> standalone,
> > so
> > > > >>> should be easy to import into the tree, and then it's just a
> matter
> > > of
> > > > >>> modifying the rest of the engine to use it. Note that my
> > > > qpid-proton-old
> > > > >>> repo is a clone of the pre-migration repo.
> > > > >>>
> > > > >>> --Rafael
> > > > >>>
> > > > >>>
> > > > >>> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
> > > > >>> rajith77@gmail.com> wrote:
> > > > >>> >
> > > > >>> > I'm starting to look at improving this areas as I was told a
> few
> > > > folks
> > > > >>> have
> > > > >>> > noted some concerns.
> > > > >>> >
> > > > >>> > I would appreciate some input on these concerns and hope to
> have
> > a
> > > > >>> > discussion to figure out how best to proceed.
> > > > >>> >
> > > > >>> > Regards,
> > > > >>> >
> > > > >>> > Rajith
> > > > >>> >
> > > > >>>
> > > > >>
> > > > >>
> > > > >
> > > >
> > >
> >
>

Re: [java] Message codec improvements

[Rafael Schloming <rh...@alum.mit.edu>]

My inclination here would be to keep things simple. One of the more
annoying aspects of working on the Java code is dealing with the fact that
all the AMQP performatives use an entirely different type system. I would
start out with developing a plain and simple POJO representation for all
the performatives defined by the protocol.

By POJO, I mean for example a Flow class where getCredit() actually returns
a standard java type (probably a long) rather than an
org.apache.qpid.proton.amqp.UnsignedInteger which I then need to convert
into a long in order to do anything useful with.

You could use some code generation here, but I wouldn't sacrifice the
naturalness of the API just to allow it to be code generated, and I
wouldn't think of this as an internal API just for the engine to use. We
should have something here that anyone could use to easily decode/encode
raw AMQP performatives in a natural way.

Once you have those classes, it should be straightforward to write a
DataHandler that will use the fast codec to produce them.

--Rafael


On Fri, Jan 23, 2015 at 12:09 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> Given the existing encorder was not put behind any sort of interface (the
> impl is directly used), it's proving to be a PIA to track down all the
> different types and changing them to use the new codec.
> Look at the "public static void register(Decoder decoder, EncoderImpl
> encoder)" method of any of the XXXType classes to get a sense of what I'm
> describing.
>
> We should take this opportunity to review the current approach and see if
> we could come up with a better alternative.
> If anybody has any suggestions, I would be happy to discuss them with you
> on the list.
>
> Regards,
>
> Rajith
>
>
>
>
> On Thu, Jan 15, 2015 at 3:15 PM, Rafael Schloming <rh...@alum.mit.edu>
> wrote:
>
> > I don't believe there is an existing one.
> >
> > --Rafael
> >
> > On Thu, Jan 15, 2015 at 3:10 PM, Rajith Muditha Attapattu <
> > rajith77@gmail.com> wrote:
> >
> > > Rafi,
> > >
> > > Do you have a JIRA for this work?
> > >
> > > Regards,
> > >
> > > Rajith
> > >
> > > On Wed, Jan 14, 2015 at 6:35 PM, Rajith Muditha Attapattu <
> > > rajith77@gmail.com> wrote:
> > >
> > > > Rafi,
> > > >
> > > > I had a closer look at the code, put it on trunk and ran your
> > benchmark.
> > > > I see quite an improvement with respect to writing lists, maps and
> > > strings.
> > > >
> > > > Simply put the writeList and writeMap methods in the old encorder is
> > > about
> > > > ~10 times slower than the new encorder.
> > > > If I run with a sufficiently large set of strings, the old encorder
> is
> > > > about ~2 times slower than the new encorder.
> > > >
> > > > I'm now focusing on hooking it up with the engine.
> > > >
> > > > Once that is done we can look at tweaking it further. But as it is,
> the
> > > > new codec is a real improvement over the existing one.
> > > > Great job Rafi!
> > > >
> > > > Rajith
> > > >
> > > > On Tue, Jan 6, 2015 at 10:28 AM, Rajith Muditha Attapattu <
> > > > rajith77@gmail.com> wrote:
> > > >
> > > >> Thanks Rafi, for the link.
> > > >> I agree that any work should use this as a basis.
> > > >>
> > > >> I plan to take a closer look at this in the next week or so.
> > > >>
> > > >> Rajith
> > > >>
> > > >> On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <rhs@alum.mit.edu
> >
> > > >> wrote:
> > > >>
> > > >>> A while back I implemented a relatively complete standalone codec
> > here:
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > >
> >
> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
> > > >>>
> > > >>> It's quite a bit faster than the existing codec. I believe any new
> > > codec
> > > >>> work should probably be based on this. It's relatively standalone,
> so
> > > >>> should be easy to import into the tree, and then it's just a matter
> > of
> > > >>> modifying the rest of the engine to use it. Note that my
> > > qpid-proton-old
> > > >>> repo is a clone of the pre-migration repo.
> > > >>>
> > > >>> --Rafael
> > > >>>
> > > >>>
> > > >>> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
> > > >>> rajith77@gmail.com> wrote:
> > > >>> >
> > > >>> > I'm starting to look at improving this areas as I was told a few
> > > folks
> > > >>> have
> > > >>> > noted some concerns.
> > > >>> >
> > > >>> > I would appreciate some input on these concerns and hope to have
> a
> > > >>> > discussion to figure out how best to proceed.
> > > >>> >
> > > >>> > Regards,
> > > >>> >
> > > >>> > Rajith
> > > >>> >
> > > >>>
> > > >>
> > > >>
> > > >
> > >
> >
>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Given the existing encorder was not put behind any sort of interface (the
impl is directly used), it's proving to be a PIA to track down all the
different types and changing them to use the new codec.
Look at the "public static void register(Decoder decoder, EncoderImpl
encoder)" method of any of the XXXType classes to get a sense of what I'm
describing.

We should take this opportunity to review the current approach and see if
we could come up with a better alternative.
If anybody has any suggestions, I would be happy to discuss them with you
on the list.

Regards,

Rajith




On Thu, Jan 15, 2015 at 3:15 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> I don't believe there is an existing one.
>
> --Rafael
>
> On Thu, Jan 15, 2015 at 3:10 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
> > Rafi,
> >
> > Do you have a JIRA for this work?
> >
> > Regards,
> >
> > Rajith
> >
> > On Wed, Jan 14, 2015 at 6:35 PM, Rajith Muditha Attapattu <
> > rajith77@gmail.com> wrote:
> >
> > > Rafi,
> > >
> > > I had a closer look at the code, put it on trunk and ran your
> benchmark.
> > > I see quite an improvement with respect to writing lists, maps and
> > strings.
> > >
> > > Simply put the writeList and writeMap methods in the old encorder is
> > about
> > > ~10 times slower than the new encorder.
> > > If I run with a sufficiently large set of strings, the old encorder is
> > > about ~2 times slower than the new encorder.
> > >
> > > I'm now focusing on hooking it up with the engine.
> > >
> > > Once that is done we can look at tweaking it further. But as it is, the
> > > new codec is a real improvement over the existing one.
> > > Great job Rafi!
> > >
> > > Rajith
> > >
> > > On Tue, Jan 6, 2015 at 10:28 AM, Rajith Muditha Attapattu <
> > > rajith77@gmail.com> wrote:
> > >
> > >> Thanks Rafi, for the link.
> > >> I agree that any work should use this as a basis.
> > >>
> > >> I plan to take a closer look at this in the next week or so.
> > >>
> > >> Rajith
> > >>
> > >> On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <rh...@alum.mit.edu>
> > >> wrote:
> > >>
> > >>> A while back I implemented a relatively complete standalone codec
> here:
> > >>>
> > >>>
> > >>>
> > >>>
> >
> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
> > >>>
> > >>> It's quite a bit faster than the existing codec. I believe any new
> > codec
> > >>> work should probably be based on this. It's relatively standalone, so
> > >>> should be easy to import into the tree, and then it's just a matter
> of
> > >>> modifying the rest of the engine to use it. Note that my
> > qpid-proton-old
> > >>> repo is a clone of the pre-migration repo.
> > >>>
> > >>> --Rafael
> > >>>
> > >>>
> > >>> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
> > >>> rajith77@gmail.com> wrote:
> > >>> >
> > >>> > I'm starting to look at improving this areas as I was told a few
> > folks
> > >>> have
> > >>> > noted some concerns.
> > >>> >
> > >>> > I would appreciate some input on these concerns and hope to have a
> > >>> > discussion to figure out how best to proceed.
> > >>> >
> > >>> > Regards,
> > >>> >
> > >>> > Rajith
> > >>> >
> > >>>
> > >>
> > >>
> > >
> >
>

Re: [java] Message codec improvements

[Rafael Schloming <rh...@alum.mit.edu>]

I don't believe there is an existing one.

--Rafael

On Thu, Jan 15, 2015 at 3:10 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> Rafi,
>
> Do you have a JIRA for this work?
>
> Regards,
>
> Rajith
>
> On Wed, Jan 14, 2015 at 6:35 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
> > Rafi,
> >
> > I had a closer look at the code, put it on trunk and ran your benchmark.
> > I see quite an improvement with respect to writing lists, maps and
> strings.
> >
> > Simply put the writeList and writeMap methods in the old encorder is
> about
> > ~10 times slower than the new encorder.
> > If I run with a sufficiently large set of strings, the old encorder is
> > about ~2 times slower than the new encorder.
> >
> > I'm now focusing on hooking it up with the engine.
> >
> > Once that is done we can look at tweaking it further. But as it is, the
> > new codec is a real improvement over the existing one.
> > Great job Rafi!
> >
> > Rajith
> >
> > On Tue, Jan 6, 2015 at 10:28 AM, Rajith Muditha Attapattu <
> > rajith77@gmail.com> wrote:
> >
> >> Thanks Rafi, for the link.
> >> I agree that any work should use this as a basis.
> >>
> >> I plan to take a closer look at this in the next week or so.
> >>
> >> Rajith
> >>
> >> On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <rh...@alum.mit.edu>
> >> wrote:
> >>
> >>> A while back I implemented a relatively complete standalone codec here:
> >>>
> >>>
> >>>
> >>>
> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
> >>>
> >>> It's quite a bit faster than the existing codec. I believe any new
> codec
> >>> work should probably be based on this. It's relatively standalone, so
> >>> should be easy to import into the tree, and then it's just a matter of
> >>> modifying the rest of the engine to use it. Note that my
> qpid-proton-old
> >>> repo is a clone of the pre-migration repo.
> >>>
> >>> --Rafael
> >>>
> >>>
> >>> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
> >>> rajith77@gmail.com> wrote:
> >>> >
> >>> > I'm starting to look at improving this areas as I was told a few
> folks
> >>> have
> >>> > noted some concerns.
> >>> >
> >>> > I would appreciate some input on these concerns and hope to have a
> >>> > discussion to figure out how best to proceed.
> >>> >
> >>> > Regards,
> >>> >
> >>> > Rajith
> >>> >
> >>>
> >>
> >>
> >
>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Rafi,

Do you have a JIRA for this work?

Regards,

Rajith

On Wed, Jan 14, 2015 at 6:35 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> Rafi,
>
> I had a closer look at the code, put it on trunk and ran your benchmark.
> I see quite an improvement with respect to writing lists, maps and strings.
>
> Simply put the writeList and writeMap methods in the old encorder is about
> ~10 times slower than the new encorder.
> If I run with a sufficiently large set of strings, the old encorder is
> about ~2 times slower than the new encorder.
>
> I'm now focusing on hooking it up with the engine.
>
> Once that is done we can look at tweaking it further. But as it is, the
> new codec is a real improvement over the existing one.
> Great job Rafi!
>
> Rajith
>
> On Tue, Jan 6, 2015 at 10:28 AM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
>
>> Thanks Rafi, for the link.
>> I agree that any work should use this as a basis.
>>
>> I plan to take a closer look at this in the next week or so.
>>
>> Rajith
>>
>> On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <rh...@alum.mit.edu>
>> wrote:
>>
>>> A while back I implemented a relatively complete standalone codec here:
>>>
>>>
>>>
>>> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
>>>
>>> It's quite a bit faster than the existing codec. I believe any new codec
>>> work should probably be based on this. It's relatively standalone, so
>>> should be easy to import into the tree, and then it's just a matter of
>>> modifying the rest of the engine to use it. Note that my qpid-proton-old
>>> repo is a clone of the pre-migration repo.
>>>
>>> --Rafael
>>>
>>>
>>> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
>>> rajith77@gmail.com> wrote:
>>> >
>>> > I'm starting to look at improving this areas as I was told a few folks
>>> have
>>> > noted some concerns.
>>> >
>>> > I would appreciate some input on these concerns and hope to have a
>>> > discussion to figure out how best to proceed.
>>> >
>>> > Regards,
>>> >
>>> > Rajith
>>> >
>>>
>>
>>
>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Rafi,

I had a closer look at the code, put it on trunk and ran your benchmark.
I see quite an improvement with respect to writing lists, maps and strings.

Simply put the writeList and writeMap methods in the old encorder is about
~10 times slower than the new encorder.
If I run with a sufficiently large set of strings, the old encorder is
about ~2 times slower than the new encorder.

I'm now focusing on hooking it up with the engine.

Once that is done we can look at tweaking it further. But as it is, the new
codec is a real improvement over the existing one.
Great job Rafi!

Rajith

On Tue, Jan 6, 2015 at 10:28 AM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:

> Thanks Rafi, for the link.
> I agree that any work should use this as a basis.
>
> I plan to take a closer look at this in the next week or so.
>
> Rajith
>
> On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <rh...@alum.mit.edu>
> wrote:
>
>> A while back I implemented a relatively complete standalone codec here:
>>
>>
>>
>> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
>>
>> It's quite a bit faster than the existing codec. I believe any new codec
>> work should probably be based on this. It's relatively standalone, so
>> should be easy to import into the tree, and then it's just a matter of
>> modifying the rest of the engine to use it. Note that my qpid-proton-old
>> repo is a clone of the pre-migration repo.
>>
>> --Rafael
>>
>>
>> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
>> rajith77@gmail.com> wrote:
>> >
>> > I'm starting to look at improving this areas as I was told a few folks
>> have
>> > noted some concerns.
>> >
>> > I would appreciate some input on these concerns and hope to have a
>> > discussion to figure out how best to proceed.
>> >
>> > Regards,
>> >
>> > Rajith
>> >
>>
>
>

Re: [java] Message codec improvements

[Rajith Muditha Attapattu <ra...@gmail.com>]

Thanks Rafi, for the link.
I agree that any work should use this as a basis.

I plan to take a closer look at this in the next week or so.

Rajith

On Wed, Dec 17, 2014 at 1:24 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> A while back I implemented a relatively complete standalone codec here:
>
>
>
> https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2
>
> It's quite a bit faster than the existing codec. I believe any new codec
> work should probably be based on this. It's relatively standalone, so
> should be easy to import into the tree, and then it's just a matter of
> modifying the rest of the engine to use it. Note that my qpid-proton-old
> repo is a clone of the pre-migration repo.
>
> --Rafael
>
>
> On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
> rajith77@gmail.com> wrote:
> >
> > I'm starting to look at improving this areas as I was told a few folks
> have
> > noted some concerns.
> >
> > I would appreciate some input on these concerns and hope to have a
> > discussion to figure out how best to proceed.
> >
> > Regards,
> >
> > Rajith
> >
>

Re: [java] Message codec improvements

[Rafael Schloming <rh...@alum.mit.edu>]

A while back I implemented a relatively complete standalone codec here:


https://github.com/rhs/qpid-proton-old/tree/codec/proton-j/src/main/java/org/apache/qpid/proton/codec2

It's quite a bit faster than the existing codec. I believe any new codec
work should probably be based on this. It's relatively standalone, so
should be easy to import into the tree, and then it's just a matter of
modifying the rest of the engine to use it. Note that my qpid-proton-old
repo is a clone of the pre-migration repo.

--Rafael


On Mon, Dec 15, 2014 at 2:17 PM, Rajith Muditha Attapattu <
rajith77@gmail.com> wrote:
>
> I'm starting to look at improving this areas as I was told a few folks have
> noted some concerns.
>
> I would appreciate some input on these concerns and hope to have a
> discussion to figure out how best to proceed.
>
> Regards,
>
> Rajith
>

Re: [java] Message codec improvements

[Clebert Suconic <cs...@redhat.com>]

I will need to recover my old-branch to give you some pointers about it…

I - A lot of the codecs are creating plenty of intermediate objects… 


Pseudo-Example on the messageCodec:  List… decodedProperties…


Message message = new Message(list.get(0), list.get(1)… );


it could be something done directly at wire layer without requiring the intermediate object. that’s putting some pressure at the GC.




There’s also a case now where having SASL is increasing the time response (having SASL is requiring having an intermediate buffer I believe)… 






And there are other cases that could go beyond simply the codec.. but I have more abstract concerns at this point.. where we could probably talk over the phone.. or have a dedicated session over IRC.






I have this transient branch containing some of the micro-benchmarks and some proposed changes I had a while ago. It’s probably not in a working state… but it would give you an idea where I was aiming for.


https://github.com/clebertsuconic/qpid-proton/tree/old-work