Cleanup: make clientParseRequest() a member of ConnStateData

This allows the HttpParser to also become a member field and persistent
across all requests on the connection instead of newely allocated on
the stack for every read cycle.
That in turn allows the parser to retain state for efficient 'trickle'
parsing across multiple read cycles.

For now the old behaviour of reset on every read is retained in order to
prevent this shuffling from causing behaviour changes. That negates most
of the actual performance gains (for now).

diff --git a/src/HttpMsg.cc b/src/HttpMsg.cc
index 9c9f5c6a908085ce54432290348cd53330eba24d..7f1db0176f41d8cb68893925fcefb67e40c7542d 100644 (file)
--- a/src/HttpMsg.cc
+++ b/src/HttpMsg.cc
@@ -434,6 +434,11 @@ HttpParserRequestLen(HttpParser *hp)
 }
 #endif
 
+HttpParser::HttpParser(const char *buf, int len)
+{
+    HttpParserInit(this, buf, len);
+}
+
 int
 HttpParser::parseRequestFirstLine()
 {

diff --git a/src/HttpMsg.h b/src/HttpMsg.h
index 736ec4fb3d5c7b87c2f3b11055fb4b76bb576192..72a932473c8f2511ef06fdf565924932b5f671a8 100644 (file)
--- a/src/HttpMsg.h
+++ b/src/HttpMsg.h
@@ -136,6 +136,9 @@ protected:
 class HttpParser
 {
 public:
+    HttpParser() {};
+    HttpParser(const char *buf, int len);
+
     /**
      * Attempt to parse the first line of a new request message.
      *

diff --git a/src/client_side.cc b/src/client_side.cc
index 6de2b4329b4bc273cfd2c14cb073585336491625..94d61be179321e3c4f835d9469e0a592ea90a527 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -192,7 +192,6 @@ static ClientSocketContext *ClientSocketContextNew(ClientHttpRequest *);
 /* other */
 static IOCB clientWriteComplete;
 static IOCB clientWriteBodyComplete;
-static bool clientParseRequest(ConnStateData * conn, bool &do_next_read);
 static PF clientLifetimeTimeout;
 static ClientSocketContext *parseHttpRequestAbort(ConnStateData * conn, const char *uri);
 static ClientSocketContext *parseHttpRequest(ConnStateData *, HttpParser *, HttpRequestMethod *, HttpVersion *);
@@ -1532,7 +1531,7 @@ ClientSocketContext::keepaliveNextRequest()
      * from our read buffer we may never re-register for another client read.
      */
 
-    if (clientParseRequest(conn, do_next_read)) {
+    if (conn->clientParseRequest(do_next_read)) {
         debugs(33, 3, "clientSocketContext::keepaliveNextRequest: FD " << conn->fd << ": parsed next request from buffer");
     }
 
@@ -2695,76 +2694,67 @@ connOkToAddRequest(ConnStateData * conn)
  * do_next_read is updated to indicate whether a read should be
  * scheduled.
  */
-static bool
-clientParseRequest(ConnStateData * conn, bool &do_next_read)
+bool
+ConnStateData::clientParseRequest(bool &do_next_read)
 {
     HttpRequestMethod method;
-    ClientSocketContext *context;
     bool parsed_req = false;
     HttpVersion http_ver;
-    HttpParser hp;
 
-    debugs(33, 5, "clientParseRequest: FD " << conn->fd << ": attempting to parse");
+    debugs(33, 5, HERE << "FD " << fd << ": attempting to parse");
 
     // Loop while we have read bytes that are not needed for producing the body
     // On errors, bodyPipe may become nil, but readMoreRequests will be cleared
-    while (conn->in.notYetUsed > 0 && !conn->bodyPipe &&
-            conn->flags.readMoreRequests) {
-        connStripBufferWhitespace (conn);
+    while (in.notYetUsed > 0 && !bodyPipe && flags.readMoreRequests) {
+        connStripBufferWhitespace(this);
 
         /* Don't try to parse if the buffer is empty */
-
-        if (conn->in.notYetUsed == 0)
+        if (in.notYetUsed == 0)
             break;
 
         /* Limit the number of concurrent requests to 2 */
-
-        if (!connOkToAddRequest(conn)) {
+        if (!connOkToAddRequest(this)) {
             break;
         }
 
         /* Should not be needed anymore */
         /* Terminate the string */
-        conn->in.buf[conn->in.notYetUsed] = '\0';
+        in.buf[in.notYetUsed] = '\0';
 
         /* Begin the parsing */
-        HttpParserInit(&hp, conn->in.buf, conn->in.notYetUsed);
-
-        /* Process request */
         PROF_start(parseHttpRequest);
+        HttpParserInit(&parser_, in.buf, in.notYetUsed);
 
-        context = parseHttpRequest(conn, &hp, &method, &http_ver);
-
+        /* Process request */
+        ClientSocketContext *context = parseHttpRequest(this, &parser_, &method, &http_ver);
         PROF_stop(parseHttpRequest);
 
         /* partial or incomplete request */
         if (!context) {
             // TODO: why parseHttpRequest can just return parseHttpRequestAbort
             // (which becomes context) but checkHeaderLimits cannot?
-            conn->checkHeaderLimits();
+            checkHeaderLimits();
             break;
         }
 
         /* status -1 or 1 */
         if (context) {
-            debugs(33, 5, "clientParseRequest: FD " << conn->fd << ": parsed a request");
-            commSetTimeout(conn->fd, Config.Timeout.lifetime, clientLifetimeTimeout,
+            debugs(33, 5, HERE << "FD " << fd << ": parsed a request");
+            commSetTimeout(fd, Config.Timeout.lifetime, clientLifetimeTimeout,
                            context->http);
 
-            clientProcessRequest(conn, &hp, context, method, http_ver);
+            clientProcessRequest(this, &parser_, context, method, http_ver);
 
-            parsed_req = true;
+            parsed_req = true; // XXX: do we really need to parse everything right NOW ?
 
             if (context->mayUseConnection()) {
-                debugs(33, 3, "clientParseRequest: Not reading, as this request may need the connection");
-                do_next_read = 0;
-                break;
+                debugs(33, 3, HERE << "Not reading, as this request may need the connection");
+                return false;
             }
         }
     }
 
     /* XXX where to 'finish' the parsing pass? */
-
     return parsed_req;
 }
 
@@ -2835,7 +2825,7 @@ ConnStateData::clientReadRequest(const CommIoCbParams &io)
     if (getConcurrentRequestCount() == 0)
         fd_note(fd, "Reading next request");
 
-    if (! clientParseRequest(this, do_next_read)) {
+    if (!clientParseRequest(do_next_read)) {
         if (!isOpen())
             return;
         /*

diff --git a/src/client_side.h b/src/client_side.h
index 3e8123894947a7d4b092fbb69c607eda8ecc6bb8..bc6e8d8ecf56812824799c2d467476e0c65b33dd 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -151,6 +151,8 @@ public:
     bool areAllContextsForThisConnection() const;
     void freeAllContexts();
     void notifyAllContexts(const int xerrno); ///< tell everybody about the err
+    /// Traffic parsing
+    bool clientParseRequest(bool &do_next_read);
     void readNextRequest();
     bool maybeMakeSpaceAvailable();
     ClientSocketContext::Pointer getCurrentContext() const;
@@ -308,6 +310,9 @@ private:
     void clientAfterReadingRequests(int do_next_read);
 
 private:
+    HttpParser parser_;
+
+    // XXX: CBDATA plays with public/private and leaves the following 'private' fields all public... :(
     CBDATA_CLASS2(ConnStateData);
     bool transparent_;
     bool closing_;