[prev in list] [next in list] [prev in thread] [next in thread] 

List:       helix-server-cvs
Subject:    [Server-cvs] protocol/rtsp crtspbase.cpp,1.22,1.23
From:       seansmith () helixcommunity ! org
Date:       2007-11-30 20:13:02
Message-ID: 200711302013.lAUKD92K018983 () mailer ! progressive-comp ! com
[Download RAW message or body]

Update of /cvsroot/server/protocol/rtsp
In directory cvs01.internal.helixcommunity.org:/tmp/cvs-serv13070/server/protocol/rtsp


Modified Files:
	crtspbase.cpp 
Log Message:
Synopsis
========
Improves message debug performance and fixes missing debug location.

Branches: HEAD, SERVER_12_RN
Reviewer: dlew


Description
===========
The AsBuffer implementation for RTSP message classes that implement IHXRTSPConsumer \
is kind of expensive. Existing proxy/server code calls AsBuffer for *every* RTSP \
message, and passes the buffer into ::HandleDebug and ::HandleTiming methods, even if \
debug/timing is disabled.

To avoid rebuilding message strings when we aren't using them, I created new \
::HandleTiming and ::HandleDebug methods that take a IHXRTSPConsumer pointer. Within \
these methods, AsBuffer is called only if debugging or timing is active, and that \
buffer is passed to the existing ::HandleXXX method.

The only side effect is that if debugging and timing are both activated, we end up \
calling AsBuffer once for each (timing and debug).

I've also noticed that wherever HandleDebug is called, HandleTiming is also called, \
and vice versa. So it might make sense to combine the methods, or call them both from \
a wrapper that checks if at least one is active (m_pTimingFile or m_pDebugFile is not \
NULL) and then calls AsBuffer.

Note that for now we need to preserve the HandleXXX methods that take buffers args \
since CRTSPBaseProtocol is still using the non-com RTSPRequestMessage class (see \
CRTSPBaseProtocol::SendRequest).


Files Affected
==============
server/protocol/rtsp/crtspbase.cpp
server/protocol/rtsp/pub/crtspbase.h
server-restricted/proxy/rproxy/rtspprxy.cpp


Testing Performed
=================
Unit Tests:
n/a

Integration Tests:
Verified core in/out messages appear in stdout when RTSPMessageDebug flag is set in \
proxy config. Verified AsBuffer usage is bypassed when message debug is disabled \
(yay!)

Leak Tests:
n/a

Performance Tests:
n/a

Platforms Tested: win32-i386-vc7
Build verified: linux-rhel4-i686, win32-i386-vc7


QA Hints
========
n/a 


Index: crtspbase.cpp
===================================================================
RCS file: /cvsroot/server/protocol/rtsp/crtspbase.cpp,v
retrieving revision 1.22
retrieving revision 1.23
diff -u -d -r1.22 -r1.23
--- crtspbase.cpp	20 Nov 2007 03:56:36 -0000	1.22
+++ crtspbase.cpp	30 Nov 2007 20:12:59 -0000	1.23
@@ -6598,6 +6598,20 @@
 
 
 void
+CRTSPBaseProtocol::HandleDebug(IHXRTSPConsumer* pCons, HXBOOL bInbound)
+{
+    IHXBuffer* pBuf = NULL;
+
+    if (m_pDebugFile != NULL)
+    {
+        pCons->AsBuffer(pBuf);
+        HandleDebug(pBuf, bInbound);
+        HX_RELEASE(pBuf);
+    }
+}
+
+
+void
 CRTSPBaseProtocol::HandleDebug(IHXBuffer* pMsgBuf, HXBOOL bInbound)
 {
     if (m_pDebugFile != NULL)
@@ -6666,6 +6680,20 @@
 
 
 void
+CRTSPBaseProtocol::HandleTiming(IHXRTSPConsumer* pCons, HXBOOL bInbound)
+{
+    IHXBuffer* pBuf = NULL;
+
+    if (m_pTimingFile != NULL)
+    {
+        pCons->AsBuffer(pBuf);
+        HandleTiming(pBuf, bInbound);
+        HX_RELEASE(pBuf);
+    }
+}
+
+
+void
 CRTSPBaseProtocol::HandleTiming(IHXBuffer* pMsgBuf, HXBOOL bInbound)
 {
     if (m_pTimingFile != NULL)
@@ -11425,7 +11453,6 @@
     IHXRTSPMessage*             pMsg = NULL;
     IHXRTSPRequestMessage*      pReq = NULL;
     IHXRTSPResponseMessage*     pRsp = NULL;
-    IHXBuffer*                  pBuf = NULL;
     UINT32                      uCSeq = 0;
     HX_RESULT rc = HXR_OK;
 
@@ -11480,11 +11507,9 @@
             return;
         }
 
-        m_pConsumer->AsBuffer(pBuf);
-        HandleDebug(pBuf, TRUE);
-        HandleTiming(pBuf, TRUE);
+        HandleDebug(m_pConsumer, TRUE);
+        HandleTiming(m_pConsumer, TRUE);
         uCSeq = pMsg->GetCSeq();
-        HX_RELEASE(pBuf);
 
         // Find session, if any
         CHXString sessionID;
@@ -11685,9 +11710,8 @@
             return;
         }
 
-        m_pConsumer->AsBuffer(pBuf);
-        HandleDebug(pBuf, TRUE);
-        HandleTiming(pBuf, TRUE);
+        HandleDebug(m_pConsumer, TRUE);
+        HandleTiming(m_pConsumer, TRUE);
         uCSeq = pMsg->GetCSeq();
 
         // is this a response to a keepalive message?  Update the session
@@ -11738,8 +11762,6 @@
             HX_DELETE(pReqMsg);
         }
 
-        HX_RELEASE(pBuf);
-
         pMsg->Release();
         pRsp->Release();
         return;


_______________________________________________
Server-cvs mailing list
Server-cvs@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/server-cvs


[prev in list] [next in list] [prev in thread] [next in thread]