----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2127 -----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h <https://reviews.apache.org/r/1980/#comment4916> How about QueueOrder? It defines the order that consumers see messages. Or perhaps something like MessageChooser /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h <https://reviews.apache.org/r/1980/#comment4917> Suggest calling this acquire() since that's what is used elsewhere to refer to assigning ownership. /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h <https://reviews.apache.org/r/1980/#comment4919> Don't put different logic in debug/release builds. Asserts are fine but this is calling a different function. If there's a bug in the NDEBUG function then developer testing won't uncover it. (even with asserts you need to be careful that they don't have side-effects, we've been bitten by that before) /trunk/qpid/cpp/src/qpid/broker/Queue.cpp <https://reviews.apache.org/r/1980/#comment4918> Shouldn't MessageAllocator replace the Messages interface? They are both supposed to store messages and determine the order in which they are acquired. Why are we sometimes choosing the next message via the MessageAllocator and other times via the Messages? Also aren't we doubling the storage? - Alan On 2011-09-27 20:26:22, Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1980/ > ----------------------------------------------------------- > > (Updated 2011-09-27 20:26:22) > > > Review request for qpid, Alan Conway and Gordon Sim. > > > Summary > ------- > > This implements QPID-3346. I'd like to get this onto trunk. > > This patch does not include optimizations I have made - I'll be reviewing & > submitting these separately as they involve more intrusive changes to the > Queue API. Ignore for now the TODO comments in the MessageGroups source files. > > Performance testing suggests that this patch has negligible effect on legacy > traffic performance (non-grouped traffic): > > current trunk: > [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count > 500000 --npubs=1 --nsubs=1 --qt=1 --summary > 58096.5 56556.7 113124 110.473 > 62659.7 62195.4 124381 121.466 > 54278.2 54136.2 108265 105.728 > 55506.3 55501 110505 107.915 > 62176.8 58019.1 115500 112.793 > Averages: > 58543.5 57281.7 114355 111.675 > > + this patch: > [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count > 500000 --npubs=1 --nsubs=1 --qt=1 --summary > 58346.1 55008.2 110510 107.92 > 59309.8 58747.5 117482 114.729 > 61323.3 58222.1 116436 113.707 > 57603 57600.4 115193 112.493 > 56091.8 56086.4 111667 109.05 > Averages: > 58534.8 57132.9 114258 111.58 > > > This addresses bug qpid-3346. > https://issues.apache.org/jira/browse/qpid-3346 > > > Diffs > ----- > > /trunk/qpid/cpp/src/CMakeLists.txt 1172628 > /trunk/qpid/cpp/src/Makefile.am 1172628 > /trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 > /trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 > /trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 > /trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 > /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 > /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 > /trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 > /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 > /trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 > /trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 > /trunk/qpid/cpp/src/tests/Makefile.am 1172628 > /trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 > /trunk/qpid/cpp/src/tests/cluster_tests.py 1172628 > /trunk/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION > /trunk/qpid/cpp/src/tests/qpid-send.cpp 1172628 > /trunk/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION > /trunk/qpid/cpp/src/tests/run_msg_group_tests_soak PRE-CREATION > /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java > 1172628 > /trunk/qpid/specs/management-schema.xml 1172628 > /trunk/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 1172628 > > Diff: https://reviews.apache.org/r/1980/diff > > > Testing > ------- > > new unit tests & tools added. Built cpp & java on Windows & various linux > distros. > > > Thanks, > > Kenneth > >
