Optimization: Faster response headers packing (#230)

Packing reply headers into StoreEntry/ShmWriter directly means numerous
tiny append() calls which involve expensive mem_node/slice searches. For
example, every two-byte ": " and CRLF delimiter is packed separately.

diff --git a/src/HttpReply.cc b/src/HttpReply.cc
index d3fe7bf04c945f836296ee7df0905a4b2105b80c..954876a02684c334b290d907a5071cb7786b1ad5 100644 (file)
--- a/src/HttpReply.cc
+++ b/src/HttpReply.cc
@@ -82,18 +82,27 @@ HttpReply::clean()
 }
 
 void
-HttpReply::packHeadersInto(Packable * p) const
+HttpReply::packHeadersUsingFastPacker(Packable &p) const
 {
-    sline.packInto(p);
-    header.packInto(p);
-    p->append("\r\n", 2);
+    sline.packInto(&p);
+    header.packInto(&p);
+    p.append("\r\n", 2);
 }
 
 void
-HttpReply::packInto(Packable * p) const
+HttpReply::packHeadersUsingSlowPacker(Packable &p) const
 {
-    packHeadersInto(p);
-    body.packInto(p);
+    MemBuf buf;
+    buf.init();
+    packHeadersUsingFastPacker(buf);
+    p.append(buf.content(), buf.contentSize());
+}
+
+void
+HttpReply::packInto(MemBuf &buf) const
+{
+    packHeadersUsingFastPacker(buf);
+    body.packInto(&buf);
 }
 
 /* create memBuf, create mem-based packer, pack, destroy packer, return MemBuf */
@@ -102,7 +111,7 @@ HttpReply::pack() const
 {
     MemBuf *mb = new MemBuf;
     mb->init();
-    packInto(mb);
+    packInto(*mb);
     return mb;
 }
 

diff --git a/src/HttpReply.h b/src/HttpReply.h
index 968a82dc5cea773080bc5a85c6dc6dc43ad1ef6c..7860ff04d77f43995a6c8d566553f9ce22632c87 100644 (file)
--- a/src/HttpReply.h
+++ b/src/HttpReply.h
@@ -100,7 +100,11 @@ public:
 
     int validatorsMatch (HttpReply const *other) const;
 
-    void packHeadersInto(Packable * p) const;
+    /// adds status line and header to the given Packable
+    /// assumes that `p` can quickly process small additions
+    void packHeadersUsingFastPacker(Packable &p) const;
+    /// same as packHeadersUsingFastPacker() but assumes that `p` cannot quickly process small additions
+    void packHeadersUsingSlowPacker(Packable &p) const;
 
     /** Clone this reply.
      *  Could be done as a copy-contructor but we do not want to accidently copy a HttpReply..
@@ -124,7 +128,7 @@ private:
 
     void hdrCacheClean();
 
-    void packInto(Packable * p) const;
+    void packInto(MemBuf &) const;
 
     /* ez-routines */
     /** \return construct 304 reply and pack it into a MemBuf */

diff --git a/src/MemStore.cc b/src/MemStore.cc
index fd1ad65fa2dbba755d8b0df810ed2feb5fff15f7..a315f7adeffa9ab1f4df59fa0410a21d2bdb641e 100644 (file)
--- a/src/MemStore.cc
+++ b/src/MemStore.cc
@@ -374,7 +374,7 @@ MemStore::updateHeadersOrThrow(Ipc::StoreMapUpdate &update)
 
     Must(update.stale.anchor);
     ShmWriter writer(*this, update.entry, update.fresh.fileNo);
-    reply.packHeadersInto(&writer);
+    reply.packHeadersUsingSlowPacker(writer);
     const uint64_t freshHdrSz = writer.totalWritten;
     debugs(20, 7, "fresh hdr_sz: " << freshHdrSz << " diff: " << (freshHdrSz - staleHdrSz));
 

diff --git a/src/store.cc b/src/store.cc
index 49013e3a0c124fa51ba96412378b14dc76cad423..0e0907e95f05fbf8d1cde22bfce0b386bb4d8e91 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -1807,7 +1807,7 @@ StoreEntry::startWriting()
     assert(rep);
 
     buffer();
-    rep->packHeadersInto(this);
+    rep->packHeadersUsingSlowPacker(*this);
     mem_obj->markEndOfReplyHeaders();
 
     rep->body.packInto(this);

diff --git a/src/tests/stub_HttpReply.cc b/src/tests/stub_HttpReply.cc
index b2f9d719019f71b15ad522f2674ad0abafd299e5..ff20c4e3c2ea578d76e0e903e38376e30482bbe9 100644 (file)
--- a/src/tests/stub_HttpReply.cc
+++ b/src/tests/stub_HttpReply.cc
@@ -18,7 +18,8 @@ HttpReply::HttpReply() : Http::Message(hoReply), date (0), last_modified (0),
 {STUB_NOP}
 HttpReply::~HttpReply() STUB
 void HttpReply::setHeaders(Http::StatusCode status, const char *reason, const char *ctype, int64_t clen, time_t lmt, time_t expires_) STUB
-void HttpReply::packHeadersInto(Packable *) const STUB
+void HttpReply::packHeadersUsingFastPacker(Packable&) const STUB
+void HttpReply::packHeadersUsingSlowPacker(Packable&) const STUB
 void HttpReply::reset() STUB
 void httpBodyPackInto(const HttpBody *, Packable *) STUB
 bool HttpReply::sanityCheckStartLine(const char *buf, const size_t hdr_len, Http::StatusCode *error) STUB_RETVAL(false)

diff --git a/src/tests/testRock.cc b/src/tests/testRock.cc
index 4eb90940f9311e8d300a2a0fff3ba8704283c97f..05d668e4ed3097c3c82274b7ad15745a2a5bf828 100644 (file)
--- a/src/tests/testRock.cc
+++ b/src/tests/testRock.cc
@@ -203,7 +203,7 @@ testRock::addEntry(const int i)
     StoreEntry *const pe = createEntry(i);
 
     pe->buffer();
-    pe->getReply()->packHeadersInto(pe);
+    pe->getReply()->packHeadersUsingSlowPacker(*pe);
     pe->flush();
     pe->timestampsSet();
     pe->complete();

diff --git a/src/tests/testUfs.cc b/src/tests/testUfs.cc
index 5ca619f96029c68fb408f0d9fac175c16a263424..aa337308d96a4754a3df55b23ab42757f6ff3225 100644 (file)
--- a/src/tests/testUfs.cc
+++ b/src/tests/testUfs.cc
@@ -152,7 +152,7 @@ testUfs::testUfsSearch()
         pe->setPublicKey();
 
         pe->buffer();
-        pe->getReply()->packHeadersInto(pe);
+        pe->getReply()->packHeadersUsingSlowPacker(*pe);
         pe->flush();
         pe->timestampsSet();
         pe->complete();