svn commit: r1612559 - in /qpid/trunk/qpid/cpp/src/qpid: broker/SessionState.cpp broker/amqp_0_10/MessageTransfer.cpp framing/FrameSet.h

[mg...@apache.org]

Author: mgoulish
Date: Tue Jul 22 12:34:26 2014
New Revision: 1612559

URL: http://svn.apache.org/r1612559
Log:
QPID-5910
The previous way of computing required credit was apparently
pretty slow -- perhaps because it is doing some  unnceessary
copying down in its guts.  (Which theory I did not prove.)
And it was running while a lock was held, which caused a
significant throughput regression (which was reported as an
enormous latency regression.)
The simpler means of calculating credit in this diff removes
most of the problem.

Modified:
    qpid/trunk/qpid/cpp/src/qpid/broker/SessionState.cpp
    qpid/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
    qpid/trunk/qpid/cpp/src/qpid/framing/FrameSet.h

Modified: qpid/trunk/qpid/cpp/src/qpid/broker/SessionState.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/SessionState.cpp?rev=1612559&r1=1612558&r2=1612559&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/SessionState.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/SessionState.cpp Tue Jul 22 12:34:26 2014
@@ -229,8 +229,9 @@ void SessionState::handleContent(AMQFram
 
         IncompleteIngressMsgXfer xfer(this, msg);
         msg->getIngressCompletion().begin();
-        semanticState.route(deliverable.getMessage(), deliverable);
+        // This call should come before routing, because it calcs required credit.
         msgBuilder.end();
+        semanticState.route(deliverable.getMessage(), deliverable);
         msg->getIngressCompletion().end(xfer);  // allows msg to complete xfer
     }
 }

Modified: qpid/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp?rev=1612559&r1=1612558&r2=1612559&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp Tue Jul 22 12:34:26 2014
@@ -153,17 +153,27 @@ uint32_t MessageTransfer::getRequiredCre
     if (cachedRequiredCredit) {
         return requiredCredit;
     } else {
-        qpid::framing::SumBodySize sum;
-        frames.map_if(sum, qpid::framing::TypeFilter2<qpid::framing::HEADER_BODY, qpid::framing::CONTENT_BODY>());
-        return sum.getSize();
+    // TODO -- remove this code and replace it with a QPID_ASSERT(cachedRequiredCredit),
+    //         then fix whatever breaks.  compute should always be called before get.
+        uint32_t sum = 0;
+        for(FrameSet::Frames::const_iterator i = frames.begin(); i != frames.end(); ++i ) {
+            uint8_t type = (*i).getBody()->type();
+            if ((type == qpid::framing::HEADER_BODY ) || (type == qpid::framing::CONTENT_BODY ))
+                sum += (*i).getBody()->encodedSize();
+        }
+        return sum;
     }
 }
 void MessageTransfer::computeRequiredCredit()
 {
     //add up payload for all header and content frames in the frameset
-    qpid::framing::SumBodySize sum;
-    frames.map_if(sum, qpid::framing::TypeFilter2<qpid::framing::HEADER_BODY, qpid::framing::CONTENT_BODY>());
-    requiredCredit = sum.getSize();
+    uint32_t sum = 0;
+    for(FrameSet::Frames::const_iterator i = frames.begin(); i != frames.end(); ++i ) {
+        uint8_t type = (*i).getBody()->type();
+        if ((type == qpid::framing::HEADER_BODY ) || (type == qpid::framing::CONTENT_BODY ))
+            sum += (*i).getBody()->encodedSize();
+    }
+    requiredCredit = sum;
     cachedRequiredCredit = true;
 }
 

Modified: qpid/trunk/qpid/cpp/src/qpid/framing/FrameSet.h
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/framing/FrameSet.h?rev=1612559&r1=1612558&r2=1612559&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/framing/FrameSet.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/framing/FrameSet.h Tue Jul 22 12:34:26 2014
@@ -37,7 +37,10 @@ namespace framing {
  */
 class FrameSet
 {
+public:
     typedef InlineVector<AMQFrame, 4> Frames;
+
+private:
     const SequenceNumber id;
     Frames parts;
     mutable uint64_t contentSize;



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org