----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11329/#review20897 -----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Message.h <https://reviews.apache.org/r/11329/#comment43030> Generally it'll be faster to use an unordered_map than a map (obviously no good if you actually need the sorted property of maps) - #include "qpid/sys/unordered_map.h" and use unordered_map<T> instead of map<T>. /trunk/qpid/cpp/src/qpid/broker/Message.h <https://reviews.apache.org/r/11329/#comment43028> sp. buy -> by /trunk/qpid/cpp/src/qpid/broker/Message.h <https://reviews.apache.org/r/11329/#comment43031> Does having 2 annotation stores give much of a speed up compared to just using 1 with a Variant? Which seems more natural (to me at least) If you are going to have a separate integer store possibly it should be of int64_t/uint64_t so it can hold any size int. /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp <https://reviews.apache.org/r/11329/#comment43041> Much of the following code seems like it isn't specific to MessageTransfer but is rather another implementation of the FieldTable encoder, doing a very similar function (encoding a map<string,Variant>). The only real difference is the use of CharSequence instead of string. There must be a less duplicative solution, possibly making the FieldTable encoder use this functionality. In any case I'd advise putting this code in it's own file as something like qpid/framing/MapEncoder.(h|cpp) where it might be used elsewhere. - Andrew Stitcher On May 22, 2013, 1:58 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11329/ > ----------------------------------------------------------- > > (Updated May 22, 2013, 1:58 p.m.) > > > Review request for qpid, Andrew Stitcher, Alan Conway, Kim van der Riet, and > Ernie Allen. > > > Description > ------- > > This attempts to reduce the performance impact of adding annotations (headers > in 0-10) to a message as it passes through the broker. At present this uses a > copy-on-write approaches that clones the header body, modifies it and then > uses that for encode or for writing to the wire. This change avoids the full > clone, by encoding the original header and while doing so adjusting for any > additions. > > Before: > > $ qpid-cpp-benchmark --repeat 5 --create-option > "node:{x-declare:{arguments:{'qpid.queue_msg_sequence':1}}}" > send-tp recv-tp l-min l-max l-avg total-tp > 30720 30671 0.32 44.03 18.28 30064 > 31778 31729 0.22 54.36 16.77 31068 > 31664 31637 0.39 69.50 22.11 31014 > 31303 31301 0.33 43.97 17.32 30737 > 31577 31403 0.33 37.73 15.59 30785 > > $ qpid-cpp-benchmark --repeat 5 > send-tp recv-tp l-min l-max l-avg total-tp > 47138 47120 0.13 29.02 9.35 45870 > 46299 46156 0.13 25.24 8.33 44803 > 45910 45793 0.13 53.53 10.81 45300 > 46607 46388 0.16 29.70 9.21 45245 > 46905 46766 0.16 31.43 9.62 45776 > > After: > > $ qpid-cpp-benchmark --repeat 5 --create-option > "node:{x-declare:{arguments:{'qpid.queue_msg_sequence':'seqno'}}}" > send-tp recv-tp l-min l-max l-avg total-tp > 44418 44376 0.15 32.29 10.24 43191 > 44127 44045 0.14 32.39 11.71 42762 > 44115 43882 0.20 37.27 13.42 42633 > 43444 43431 0.15 36.99 12.00 42147 > 44573 44265 0.17 34.87 11.81 42898 > > $ qpid-cpp-benchmark --repeat 5 > send-tp recv-tp l-min l-max l-avg total-tp > 47178 47061 0.16 27.23 8.46 45644 > 46291 46291 0.14 25.92 9.51 44812 > 47704 47525 0.17 32.19 9.06 45998 > 47771 47745 0.17 21.34 8.60 46271 > 43950 43931 0.13 52.13 10.60 43118 > > So, there is still some impact, but I believe it has been reduced. There are > some further things to try (caching encoded header as originally received to > avoid re-encoding it). > > > Diffs > ----- > > /trunk/qpid/cpp/src/CMakeLists.txt 1485001 > /trunk/qpid/cpp/src/Makefile.am 1485001 > /trunk/qpid/cpp/src/qpid/broker/Message.h 1485001 > /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1485001 > /trunk/qpid/cpp/src/qpid/broker/PersistableMessage.h 1485001 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1485001 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1485001 > /trunk/qpid/cpp/src/qpid/broker/SessionState.h 1485001 > /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1485001 > /trunk/qpid/cpp/src/qpid/broker/amqp/Message.h 1485001 > /trunk/qpid/cpp/src/qpid/broker/amqp/Message.cpp 1485001 > /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.h 1485001 > /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp 1485001 > /trunk/qpid/cpp/src/qpid/framing/AMQFrame.h 1485001 > /trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp 1485001 > /trunk/qpid/cpp/src/qpid/framing/FrameSet.h 1485001 > /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/framing/RawFrameBody.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/11329/diff/ > > > Testing > ------- > > make check/test passes both on autotools and cmake builds > > > Thanks, > > Gordon Sim > >
