---------------------
PatchSet 11207 
Date: 2007/01/26 02:33:12
Author: adrian
Branch: HEAD
Tag: (none) 
Log:
Remove the parsing of the reply buffer in client-side; clone the mem->reply instead

mem->reply should exist after the first storeClientCopy() from position 0 has
executed. All the existing client-side code makes this assumption and will try
parsing the initial buffer at offset 0.

Implement a httpReplyClone() routine which just copies all the relevant data
from one reply into another and use this in client_side::clientBuildReply() rather
than re-parsing the same buffer used to create mem->reply in the first place..

(Luckily for us, mem->reply is created -once- for an in-memory object and persists
for as long as the MemObject does. This means HITs don't incur a reply parsing
penalty. Quite nifty. The initial push of header data from http.c (where its parsed)
is re-parsed in the store client routines, but we can use httpReplyClone there too
later on to remove that overhead as well.

Members: 
	src/HttpReply.c:1.58->1.59 
	src/client_side.c:1.696->1.697 
	src/protos.h:1.523->1.524 
	src/store.c:1.572->1.573 
	src/store_client.c:1.123->1.124 

Index: squid/src/HttpReply.c
===================================================================
RCS file: /cvsroot/squid/squid/src/HttpReply.c,v
retrieving revision 1.58
retrieving revision 1.59
diff -u -r1.58 -r1.59
--- squid/src/HttpReply.c	25 Jan 2007 00:12:43 -0000	1.58
+++ squid/src/HttpReply.c	26 Jan 2007 02:33:12 -0000	1.59
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpReply.c,v 1.58 2007/01/25 00:12:43 wessels Exp $
+ * $Id: HttpReply.c,v 1.59 2007/01/26 02:33:12 adrian Exp $
  *
  * DEBUG: section 58    HTTP Reply (Response)
  * AUTHOR: Alex Rousskov
@@ -349,6 +349,41 @@
     rep->expires = httpReplyHdrExpirationTime(rep);
 }
 
+HttpReply *
+httpReplyClone(HttpReply * src)
+{
+    HttpReply *dst = httpReplyCreate();
+
+    /* basic variables */
+    dst->hdr_sz = src->hdr_sz;
+    dst->content_length = src->content_length;
+    dst->date = src->date;
+    dst->last_modified = src->last_modified;
+    dst->expires = src->expires;
+
+    /* parser state */
+    dst->pstate = src->pstate;
+    /* status line */
+    dst->sline = src->sline;
+    /* header */
+    httpHeaderAppend(&dst->header, &src->header);
+    /* body, if applicable */
+    if (dst->body.mb.buf != NULL)
+	memBufAppend(&dst->body.mb, dst->body.mb.buf, dst->body.mb.size);
+
+    /*
+     * The next two are a bit .. special. I hate delving into the headers
+     * when we've already -done- that, but I'll worry about doing it
+     * faster later. Besides, there's too much other code to fix up.
+     */
+    /* cache control */
+    dst->cache_control = httpHeaderGetCc(&dst->header);
+    /* content range */
+    dst->content_range = httpHeaderGetContRange(&dst->header);
+
+    return dst;
+}
+
 /* sync this routine when you update HttpReply struct */
 static void
 httpReplyHdrCacheClean(HttpReply * rep)
Index: squid/src/client_side.c
===================================================================
RCS file: /cvsroot/squid/squid/src/client_side.c,v
retrieving revision 1.696
retrieving revision 1.697
diff -u -r1.696 -r1.697
--- squid/src/client_side.c	26 Jan 2007 01:51:24 -0000	1.696
+++ squid/src/client_side.c	26 Jan 2007 02:33:12 -0000	1.697
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.c,v 1.696 2007/01/26 01:51:24 hno Exp $
+ * $Id: client_side.c,v 1.697 2007/01/26 02:33:12 adrian Exp $
  *
  * DEBUG: section 33    Client-side Routines
  * AUTHOR: Duane Wessels
@@ -1994,12 +1994,39 @@
     httpHdrMangleList(hdr, request);
 }
 
+/* Used exclusively by clientBuildReply() during failure cases only */
+static void
+clientUnwindReply(clientHttpRequest * http, HttpReply * rep)
+{
+    if (rep != NULL) {
+	httpReplyDestroy(rep);
+	rep = NULL;
+    }
+    /* This destroys the range request */
+    if (http->request->range)
+	clientBuildRangeHeader(http, rep);
+}
+
+/*
+ * This routine was historically called when we think we've got enough header
+ * data - ie, after the first read. The store would not be allowed to release
+ * data to be read until after all the headers were appended.
+ *
+ * So we, for now, just assume all the headers are here or they won't ever
+ * be.
+ */
 static HttpReply *
 clientBuildReply(clientHttpRequest * http, const char *buf, size_t size)
 {
-    HttpReply *rep = httpReplyCreate();
-    size_t k = headersEnd(buf, size);
-    if (k && httpReplyParse(rep, buf, k)) {
+    HttpReply *rep = NULL;
+    /* If we don't have a memobj / reply by now then we're stuffed */
+    if (http->sc->entry->mem_obj == NULL || http->sc->entry->mem_obj->reply == NULL) {
+	clientUnwindReply(http, NULL);
+	return NULL;
+    }
+    /* try to grab the already-parsed header */
+    rep = httpReplyClone(http->sc->entry->mem_obj->reply);
+    if (rep->sline.status == psParsed) {
 	/* enforce 1.0 reply version */
 	httpBuildVersion(&rep->sline.version, 1, 0);
 	/* do header conversions */
@@ -2010,13 +2037,8 @@
 		HTTP_PARTIAL_CONTENT, NULL);
     } else {
 	/* parsing failure, get rid of the invalid reply */
-	httpReplyDestroy(rep);
-	rep = NULL;
-	/* if we were going to do ranges, backoff */
-	if (http->request->range) {
-	    /* this will fail and destroy request->range */
-	    clientBuildRangeHeader(http, rep);
-	}
+	clientUnwindReply(http, rep);
+	return NULL;
     }
     return rep;
 }
@@ -2084,7 +2106,7 @@
     assert(size > 0);
     mem = e->mem_obj;
     assert(!EBIT_TEST(e->flags, ENTRY_ABORTED));
-    if (mem->reply->sline.status == 0) {
+    if (!memHaveHeaders(mem)) {
 	/*
 	 * we don't have full reply headers yet; either wait for more or
 	 * punt to clientProcessMiss.
Index: squid/src/protos.h
===================================================================
RCS file: /cvsroot/squid/squid/src/protos.h,v
retrieving revision 1.523
retrieving revision 1.524
diff -u -r1.523 -r1.524
--- squid/src/protos.h	26 Jan 2007 01:51:24 -0000	1.523
+++ squid/src/protos.h	26 Jan 2007 02:33:12 -0000	1.524
@@ -1,6 +1,6 @@
 
 /*
- * $Id: protos.h,v 1.523 2007/01/26 01:51:24 hno Exp $
+ * $Id: protos.h,v 1.524 2007/01/26 02:33:12 adrian Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -506,6 +506,7 @@
 extern int httpReplyHasCc(const HttpReply * rep, http_hdr_cc_type type);
 extern void httpRedirectReply(HttpReply *, http_status, const char *);
 extern squid_off_t httpReplyBodySize(method_t, const HttpReply *);
+extern HttpReply *httpReplyClone(HttpReply * src);
 
 /* Http Request */
 extern request_t *requestCreate(method_t, protocol_t, const char *urlpath);
@@ -956,6 +957,8 @@
 void storeDeferRead(StoreEntry *, int fd);
 void storeResumeRead(StoreEntry *);
 void storeResetDefer(StoreEntry *);
+extern int memHaveHeaders(const MemObject * mem);
+
 
 /* store_modules.c */
 extern void storeFsSetup(void);
Index: squid/src/store.c
===================================================================
RCS file: /cvsroot/squid/squid/src/store.c,v
retrieving revision 1.572
retrieving revision 1.573
diff -u -r1.572 -r1.573
--- squid/src/store.c	26 Jan 2007 00:44:01 -0000	1.572
+++ squid/src/store.c	26 Jan 2007 02:33:12 -0000	1.573
@@ -1,6 +1,6 @@
 
 /*
- * $Id: store.c,v 1.572 2007/01/26 00:44:01 adrian Exp $
+ * $Id: store.c,v 1.573 2007/01/26 02:33:12 adrian Exp $
  *
  * DEBUG: section 20    Storage Manager
  * AUTHOR: Harvest Derived
@@ -125,6 +125,19 @@
     return mem;
 }
 
+int
+memHaveHeaders(const MemObject * mem)
+{
+    if (mem->reply == NULL)
+	return 0;
+    if (mem->reply->sline.status == 0)
+	return 0;
+    if (mem->reply->pstate != psParsed)
+	return 0;
+    return 1;
+}
+
+
 StoreEntry *
 new_StoreEntry(int mem_obj_flag, const char *url)
 {
Index: squid/src/store_client.c
===================================================================
RCS file: /cvsroot/squid/squid/src/store_client.c,v
retrieving revision 1.123
retrieving revision 1.124
diff -u -r1.123 -r1.124
--- squid/src/store_client.c	5 Jun 2006 22:47:01 -0000	1.123
+++ squid/src/store_client.c	26 Jan 2007 02:33:12 -0000	1.124
@@ -1,6 +1,6 @@
 
 /*
- * $Id: store_client.c,v 1.123 2006/06/05 22:47:01 hno Exp $
+ * $Id: store_client.c,v 1.124 2007/01/26 02:33:12 adrian Exp $
  *
  * DEBUG: section 20    Storage Manager Client-Side Interface
  * AUTHOR: Duane Wessels
@@ -363,7 +363,7 @@
     sc->flags.disk_io_pending = 0;
     assert(sc->callback != NULL);
     debug(20, 3) ("storeClientReadBody: len %d\n", (int) len);
-    if (sc->copy_offset == 0 && len > 0 && mem->reply->sline.status == 0)
+    if (sc->copy_offset == 0 && len > 0 && memHaveHeaders(mem) == 0)
 	httpReplyParse(mem->reply, sc->copy_buf, headersEnd(sc->copy_buf, len));
     storeClientCallback(sc, len);
 }
@@ -471,7 +471,7 @@
 	debug(20, 3) ("storeClientReadHeader: copying %d bytes of body\n",
 	    (int) copy_sz);
 	xmemmove(sc->copy_buf, sc->copy_buf + swap_hdr_sz, copy_sz);
-	if (sc->copy_offset == 0 && len > 0 && mem->reply->sline.status == 0)
+	if (sc->copy_offset == 0 && len > 0 && memHaveHeaders(mem) == 0)
 	    httpReplyParse(mem->reply, sc->copy_buf,
 		headersEnd(sc->copy_buf, copy_sz));
 	storeClientCallback(sc, copy_sz);