----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21483/#review43362 -----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Message.h <https://reviews.apache.org/r/21483/#comment77448> This doesn't seem right to me: The previous Encoding was an interface (pure abstract class) and this change has given it state. I think maybe you should add extra member functions to the interface but use the SharedState implementation class as a mixin to the actual leaf implementation classes rather than mixing in to this interface. /trunk/qpid/cpp/src/qpid/broker/Message.h <https://reviews.apache.org/r/21483/#comment77449> I don't think this Optional has much advantage over just using a simple boost::scoped_ptr. The only difference is explicit vs implicit copying. If you really want to keep it using a boost::scoped_ptr in there would reduce it to just a few lines. - Andrew Stitcher On May 15, 2014, 11:45 a.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21483/ > ----------------------------------------------------------- > > (Updated May 15, 2014, 11:45 a.m.) > > > Review request for qpid, Alan Conway, Andrew Stitcher, and Pavel Moravec. > > > Repository: qpid > > > Description > ------- > > Previous releases (e.g. 0.18) used to shared almost all message state. This > was not correct and the current codebase only shares the incoming message as > read off the wire. However, this leads to higher memory consumption in the > fanout case. > > There are some optimisations we could make to that. Two different changes are > included in this patch for review and comment. With these both in place, 1000 > messages shared between 500 queues dropped from taking around 81k to taking > around 43k as compared with the 0.18 release which took around 27k. > > The two changes are: > > (1) share some of the immutable items that don't arrive over the wire but > don't need to be changed per queue, e.g. expiration and publisher > > (2) make the annotations map (which seems to take a large amount of memory > even when empty) optional using a custom template (the boost optional > template doesn't save any memory). > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/Message.h 1594633 > /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1594633 > > Diff: https://reviews.apache.org/r/21483/diff/ > > > Testing > ------- > > make test passes > > > Thanks, > > Gordon Sim > >
