basic_parser optimizations

CHANGELOG.md

@@ -1,3 +1,9 @@
+Version 69:
+
+* basic_parser optimizations
+
+--------------------------------------------------------------------------------
+
 Version 68:
 
 * Split common tests to a new project

include/beast/http/detail/basic_parser.hpp

@@ -12,6 +12,7 @@
 #include <beast/http/error.hpp>
 #include <beast/http/detail/rfc7230.hpp>
 #include <boost/version.hpp>
+#include <algorithm>
 #include <cstddef>
 #include <utility>
 
@@ -276,7 +277,7 @@ protected:
     bool
     parse_crlf(char const*& it)
     {
-        if( it[0] != '\r' && it[1] != '\n')
+        if( it[0] != '\r' || it[1] != '\n')
             return false;
         it += 2;
         return true;
@@ -381,10 +382,31 @@ protected:
     static
     char const*
     find_eol(
-        char const* first, char const* last,
+        char const* it, char const* last,
             error_code& ec)
     {
-        auto it = first;
+#if 0
+        // SLOWER
+        it = reinterpret_cast<char const*>(
+            std::memchr(it, '\r', last - it));
+        if(! it)
+        {
+            ec.assign(0, ec.category());
+            return nullptr;
+        }
+        if(it + 2 > last)
+        {
+            ec.assign(0, ec.category());
+            return nullptr;
+        }
+        if(it[1] != '\n')
+        {
+            ec = error::bad_line_ending;
+            return nullptr;
+        }
+        ec.assign(0, ec.category());
+        return it + 2;
+#else
         for(;;)
         {
             if(it == last)
@@ -411,61 +433,41 @@ protected:
             // for lines terminated with a single '\n'?
             ++it;
         }
+#endif
     }
 
     // VFALCO Can SIMD help this?
     static
     char const*
-    find_eom(
+    find_eom(char const* p, char const* last)
-        char const* first, char const* last,
-            error_code& ec)
     {
-        auto it = first;
         for(;;)
         {
-            if(it == last)
+            if(p + 4 > last)
-            {
-                ec.assign(0, ec.category());
                 return nullptr;
-            }
+            if(p[3] != '\n')
-            if(*it == '\r')
             {
-                if(++it == last)
+                if(p[3] == '\r')
+                    ++p;
+                else
+                    p += 4;
+            }
+            else if(p[2] != '\r')
             {
-                    ec.assign(0, ec.category());
+                p += 4;
-                    return nullptr;
             }
-                if(*it != '\n')
+            else if(p[1] != '\n')
             {
-                    ec = error::bad_line_ending;
+                p += 2;
-                    return nullptr;
             }
-                if(++it == last)
+            else if(p[0] != '\r')
             {
-                    ec.assign(0, ec.category());
+                p += 2;
-                    return nullptr;
             }
-                if(*it != '\r')
+            else
             {
-                    ++it;
+                return p + 4;
-                    continue;
             }
-                if(++it == last)
-                {
-                    ec.assign(0, ec.category());
-                    return nullptr;
-                }
-                if(*it != '\n')
-                {
-                    ec = error::bad_line_ending;
-                    return nullptr;
-                }
-                ec.assign(0, ec.category());
-                return ++it;
-            }
-            // VFALCO Should we handle the legacy case
-            // for lines terminated with a single '\n'?
-            ++it;
         }
     }
 };

include/beast/http/impl/basic_parser.ipp

@@ -21,6 +21,40 @@
 namespace beast {
 namespace http {
 
+namespace detail {
+
+template<class FwdIt>
+inline
+FwdIt
+skip_ows2(FwdIt it, FwdIt const& end)
+{
+    while(it != end)
+    {
+        if(*it != ' ' && *it != '\t')
+            break;
+        ++it;
+    }
+    return it;
+}
+
+template<class RanIt>
+inline
+RanIt
+skip_ows_rev2(
+    RanIt it, RanIt const& first)
+{
+    while(it != first)
+    {
+        auto const c = it[-1];
+        if(c != ' ' && c != '\t')
+            break;
+        --it;
+    }
+    return it;
+}
+
+} // detail
+
 template<bool isRequest, class Derived>
 template<class OtherDerived>
 basic_parser<isRequest, Derived>::
@@ -265,10 +299,8 @@ parse_header(char const*& p,
         ec = http::error::need_more;
         return;
     }
-    auto const term = find_eom(
+    auto const term =
-        p + skip_, p + n, ec);
+        find_eom(p + skip_, p + n);
-    if(ec)
-        return;
     if(! term)
     {
         skip_ = n - 3;
@@ -328,7 +360,13 @@ parse_header(char const*& p, char const* term,
     }
 
     auto const version = parse_version(p);
-    if(version < 0 || ! parse_crlf(p))
+    if(version < 0)
+    {
+        ec = error::bad_version;
+        return;
+    }
+
+    if(! parse_crlf(p))
     {
         ec = error::bad_version;
         return;
@@ -595,9 +633,7 @@ parse_chunk_header(char const*& p0,
         BOOST_ASSERT(! ec);
     }
 
-    auto eom = find_eom(p0 + skip_, pend, ec);
+    auto eom = find_eom(p0 + skip_, pend);
-    if(ec)
-        return;
     if(! eom)
     {
         BOOST_ASSERT(n >= 3);
@@ -688,8 +724,8 @@ parse_fields(char const*& p,
            *term != '\t')
         {
             auto it2 = term - 2;
-            detail::skip_ows(p, it2);
+            p = detail::skip_ows2(p, it2);
-            detail::skip_ows_rev(it2, p);
+            it2 = detail::skip_ows_rev2(it2, p);
             auto const f = string_to_field(name);
             auto const value = make_string(p, it2);
             do_field(f, value, ec);
@@ -706,7 +742,7 @@ parse_fields(char const*& p,
             for(;;)
             {
                 auto const it2 = term - 2;
-                detail::skip_ows(p, it2);
+                p = detail::skip_ows2(p, it2);
                 if(p != it2)
                     break;
                 p = term;
@@ -729,7 +765,7 @@ parse_fields(char const*& p,
                         if(*p != ' ' && *p != '\t')
                             break;
                         s.push_back(' ');
-                        detail::skip_ows(p, term - 2);
+                        p = detail::skip_ows2(p, term - 2);
                         term = find_eol(p, last, ec);
                         if(ec)
                             return;

test/http/basic_parser.cpp

@@ -378,8 +378,8 @@ public:
         bad<true>("GET / HTTP/1.0 \r\n"     "\r\n",     error::bad_version);
         bad<true>("GET / HTTP/1_0\r\n"      "\r\n",     error::bad_version);
         bad<true>("GET / HTTP/1.0\n\r\n"    "\r\n",     error::bad_version);
-        bad<true>("GET / HTTP/1.0\n\r\r\n"  "\r\n",     error::bad_line_ending);
+        bad<true>("GET / HTTP/1.0\n\r\r\n"  "\r\n",     error::bad_version);
-        bad<true>("GET / HTTP/1.0\r\r\n"    "\r\n",     error::bad_line_ending);
+        bad<true>("GET / HTTP/1.0\r\r\n"    "\r\n",     error::bad_version);
     }
 
     void
@@ -403,7 +403,7 @@ public:
         good<false>("HTTP/1.1 200 \x80\x81...\xfe\xff\r\n\r\n");
         good<false>("HTTP/1.1 200 !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\r\n\r\n");
 
-        bad<false>("\rHTTP/1.0 200 OK\r\n"  "\r\n",     error::bad_line_ending);
+        bad<false>("\rHTTP/1.0 200 OK\r\n"  "\r\n",     error::bad_version);
         bad<false>("\nHTTP/1.0 200 OK\r\n"  "\r\n",     error::bad_version);
         bad<false>(" HTTP/1.0 200 OK\r\n"   "\r\n",     error::bad_version);
         bad<false>("_TTP/1.0 200 OK\r\n"    "\r\n",     error::bad_version);
@@ -426,7 +426,7 @@ public:
         bad<false>("HTTP/1.0 200 \x01\r\n"  "\r\n",     error::bad_reason);
         bad<false>("HTTP/1.0 200 \x7f\r\n"  "\r\n",     error::bad_reason);
         bad<false>("HTTP/1.0 200 OK\n\r\n"  "\r\n",     error::bad_reason);
-        bad<false>("HTTP/1.0 200 OK\r\r\n"  "\r\n",     error::bad_line_ending);
+        bad<false>("HTTP/1.0 200 OK\r\r\n"  "\r\n",     error::bad_reason);
     }
 
     void