From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Thu, 26 Jul 2018 17:44:14 +0000 (+0000)
Subject: Optimization: Faster response headers packing (#230)
X-Git-Tag: M-staged-PR230
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f8432432a77535cd4ec0d34fff716e4bf7f467b2;p=thirdparty%2Fsquid.git

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 d3fe7bf04c..954876a026 100644
--- 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 968a82dc5c..7860ff04d7 100644
--- 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 fd1ad65fa2..a315f7adef 100644
--- 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 49013e3a0c..0e0907e95f 100644
--- 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 b2f9d71901..ff20c4e3c2 100644
--- 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 4eb90940f9..05d668e4ed 100644
--- 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 5ca619f960..aa337308d9 100644
--- 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();