Various micro perf fixes and cleanup found while implementing CN overrides so far (microsoft#818)

* Microperf: Use lock_guard instead of unique_lock.

* Bill hates lock_guard more.

* Ref them connections.

* Demacroize.

* Commonize port and merge some CRLFs into adjacent string literals.

* Add missing template keyword.

* Avoid stringstream in constructing the proxy_str.

* Remove unused forward declarations of some HTTP things.

* Use NOMINMAX instead of undef min and max everywhere.

* Bunch of inheritance hygiene.

* What do you mean you want examples to compile?

* More CR comments from Robert.

* Add static.

* Use existing to_string_t.

Author: BillyONeal

Release/include/cpprest/asyncrt_utils.h

@@ -228,7 +228,7 @@ namespace conversions
 
 #if defined(__ANDROID__)
         template<class T>
-        inline std::string to_string(const T& t)
+        inline std::string to_string(const T t)
         {
             std::ostringstream os;
             os.imbue(std::locale::classic());
@@ -238,16 +238,16 @@ namespace conversions
 #endif
 
         template<class T>
-        inline utility::string_t to_string_t(T&& t)
+        inline utility::string_t to_string_t(const T t)
         {
 #ifdef _UTF16_STRINGS
             using std::to_wstring;
-            return to_wstring(std::forward<T>(t));
+            return to_wstring(t);
 #else
 #if !defined(__ANDROID__)
             using std::to_string;
 #endif
-            return to_string(std::forward<T>(t));
+            return to_string(t);
 #endif
         }
 

Release/include/cpprest/details/web_utilities.h

@@ -14,17 +14,6 @@
 
 namespace web
 {
-
-namespace http { namespace client { namespace details {
-class winhttp_client;
-class winrt_client;
-class asio_context;
-}}}
-namespace websockets { namespace client { namespace details {
-class winrt_callback_client;
-class wspp_callback_client;
-}}}
-
 namespace details
 {
 

Release/include/cpprest/streams.h

@@ -1748,7 +1748,7 @@ class type_parser<CharType, std::enable_if_t<sizeof(CharType) == 1, std::basic_s
 
     static pplx::task<std::wstring> parse(streams::streambuf<CharType> buffer)
     {
-        return base::_parse_input<std::basic_string<char>,std::basic_string<wchar_t>>(buffer, _accept_char, _extract_result);
+        return base::template _parse_input<std::basic_string<char>,std::basic_string<wchar_t>>(buffer, _accept_char, _extract_result);
     }
 
 private:

Release/src/http/client/http_client_asio.cpp

@@ -15,9 +15,6 @@
 
 #include "stdafx.h"
 
-#undef min
-#undef max
-
 #include "cpprest/asyncrt_utils.h"
 #include "../common/internal_http_helpers.h"
 
@@ -83,7 +80,7 @@ using utility::conversions::details::to_string;
 using std::to_string;
 #endif
 
-#define CRLF std::string("\r\n")
+static const std::string CRLF("\r\n");
 
 namespace web { namespace http
 {
@@ -213,14 +210,15 @@ class asio_connection
     template <typename Iterator, typename Handler>
     void async_connect(const Iterator &begin, const Handler &handler)
     {
-        std::unique_lock<std::mutex> lock(m_socket_lock);
-        if (!m_closed)
-            m_socket.async_connect(begin, handler);
-        else
         {
-            lock.unlock();
-            handler(boost::asio::error::operation_aborted);
-        }
+            std::lock_guard<std::mutex> lock(m_socket_lock);
+            if (!m_closed) {
+                m_socket.async_connect(begin, handler);
+                return;
+            }
+        }   // unlock
+
+        handler(boost::asio::error::operation_aborted);
     }
 
     template <typename HandshakeHandler, typename CertificateHandler>
@@ -339,7 +337,7 @@ class asio_connection
 ///   }
 /// </code>
 /// </remarks>
-class asio_connection_pool : public std::enable_shared_from_this<asio_connection_pool>
+class asio_connection_pool final : public std::enable_shared_from_this<asio_connection_pool>
 {
 public:
     asio_connection_pool() : m_pool_epoch_timer(crossplat::threadpool::shared_instance().service())
@@ -396,24 +394,25 @@ class asio_connection_pool : public std::enable_shared_from_this<asio_connection
             if (!pool)
                 return;
             auto& self = *pool;
+            auto& connections = self.m_connections;
 
             std::lock_guard<std::mutex> lock(self.m_lock);
             if (self.m_prev_epoch == self.m_epoch)
             {
-                self.m_connections.clear();
+                connections.clear();
                 self.is_timer_running = false;
                 return;
             }
             else
             {
                 auto prev_epoch = self.m_prev_epoch;
-                auto erase_end = std::find_if(self.m_connections.begin(), self.m_connections.end(),
+                auto erase_end = std::find_if(connections.begin(), connections.end(),
                     [prev_epoch](std::pair<uint64_t, std::shared_ptr<asio_connection>>& p)
                 {
                     return p.first > prev_epoch;
                 });
 
-                self.m_connections.erase(self.m_connections.begin(), erase_end);
+                connections.erase(connections.begin(), erase_end);
                 start_epoch_interval(pool);
             }
         });
@@ -438,7 +437,7 @@ class asio_client final : public _http_client_communicator
         , m_start_with_ssl(base_uri().scheme() == U("https") && !this->client_config().proxy().is_specified())
     {}
 
-    void send_request(const std::shared_ptr<request_context> &request_ctx) override;
+    virtual void send_request(const std::shared_ptr<request_context> &request_ctx) override;
 
     void release_connection(std::shared_ptr<asio_connection>& conn)
     {
@@ -468,7 +467,7 @@ class asio_client final : public _http_client_communicator
     const bool m_start_with_ssl;
 };
 
-class asio_context : public request_context, public std::enable_shared_from_this<asio_context>
+class asio_context final : public request_context, public std::enable_shared_from_this<asio_context>
 {
     friend class asio_client;
 public:
@@ -501,7 +500,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
         return ctx;
     }
 
-    class ssl_proxy_tunnel : public std::enable_shared_from_this<ssl_proxy_tunnel>
+    class ssl_proxy_tunnel final : public std::enable_shared_from_this<ssl_proxy_tunnel>
     {
     public:
         ssl_proxy_tunnel(std::shared_ptr<asio_context> context, std::function<void(std::shared_ptr<asio_context>)> ssl_tunnel_established)
@@ -518,14 +517,15 @@ class asio_context : public request_context, public std::enable_shared_from_this
 
             const auto &base_uri = m_context->m_http_client->base_uri();
             const auto &host = utility::conversions::to_utf8string(base_uri.host());
-            const auto &port = base_uri.port();
+            const int portRaw = base_uri.port();
+            const int port = (portRaw != 0) ? portRaw : 443;
 
             std::ostream request_stream(&m_request);
             request_stream.imbue(std::locale::classic());
 
-            request_stream << "CONNECT " << host << ":" << ((port != 0) ? port : 443) << " HTTP/1.1" << CRLF;
-            request_stream << "Host: " << host << ":" << ((port != 0) ? port : 443) << CRLF;
-            request_stream << "Proxy-Connection: Keep-Alive" << CRLF;
+            request_stream << "CONNECT " << host << ":" << port << " HTTP/1.1\r\n";
+            request_stream << "Host: " << host << ":" << port << CRLF;
+            request_stream << "Proxy-Connection: Keep-Alive\r\n";
 
             if(m_context->m_http_client->client_config().proxy().credentials().is_set())
             {
@@ -698,7 +698,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
             request_stream.imbue(std::locale::classic());
             const auto &host = utility::conversions::to_utf8string(base_uri.host());
 
-            request_stream << utility::conversions::to_utf8string(method) << " " << utility::conversions::to_utf8string(encoded_resource) << " " << "HTTP/1.1" << CRLF;
+            request_stream << utility::conversions::to_utf8string(method) << " " << utility::conversions::to_utf8string(encoded_resource) << " " << "HTTP/1.1\r\n";
 
             int port = base_uri.port();
 
@@ -766,7 +766,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
             request_stream << utility::conversions::to_utf8string(::web::http::details::flatten_http_headers(ctx->m_request.headers()));
             request_stream << extra_headers;
             // Enforce HTTP connection keep alive (even for the old HTTP/1.0 protocol).
-            request_stream << "Connection: Keep-Alive" << CRLF << CRLF;
+            request_stream << "Connection: Keep-Alive\r\n\r\n";
 
             // Start connection timeout timer.
             if (!ctx->m_timer.has_started())
@@ -1345,7 +1345,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
             }
             else
             {
-                async_read_until_buffersize(octets + CRLF.size(), // + 2 for crlf
+                async_read_until_buffersize(octets + CRLF.size(),
                                             boost::bind(&asio_context::handle_chunk, shared_from_this(), boost::asio::placeholders::error, octets));
             }
         }

Release/src/http/client/http_client_winhttp.cpp

@@ -23,9 +23,6 @@
 #include <VersionHelpers.h>
 #endif
 
-#undef min
-#undef max
-
 namespace web
 {
 namespace http
@@ -185,7 +182,7 @@ enum msg_body_type
 };
 
 // Additional information necessary to track a WinHTTP request.
-class winhttp_request_context : public request_context
+class winhttp_request_context final : public request_context
 {
 public:
 
@@ -248,7 +245,7 @@ class winhttp_request_context : public request_context
     std::shared_ptr<winhttp_request_context> m_self_reference;
     memory_holder m_body_data;
 
-    virtual void cleanup()
+    void cleanup()
     {
         if(m_request_handle != nullptr)
         {
@@ -260,7 +257,7 @@ class winhttp_request_context : public request_context
 
 protected:
 
-    virtual void finish()
+    virtual void finish() override
     {
         request_context::finish();
         assert(m_self_reference != nullptr);
@@ -346,7 +343,7 @@ struct ie_proxy_config : WINHTTP_CURRENT_USER_IE_PROXY_CONFIG
 };
 
 // WinHTTP client.
-class winhttp_client : public _http_client_communicator
+class winhttp_client final : public _http_client_communicator
 {
 public:
     winhttp_client(http::uri address, http_client_config client_config)
@@ -502,17 +499,13 @@ class winhttp_client : public _http_client_communicator
             }
             else
             {
+                proxy_str = uri.host();
                 if (uri.port() > 0)
                 {
-                    utility::ostringstream_t ss;
-                    ss.imbue(std::locale::classic());
-                    ss << uri.host() << _XPLATSTR(":") << uri.port();
-                    proxy_str = ss.str();
-                }
-                else
-                {
-                    proxy_str = uri.host();
+                    proxy_str.push_back(_XPLATSTR(':'));
+                    proxy_str.append(::utility::conversions::details::to_string_t(uri.port()));
                 }
+
                 proxy_name = proxy_str.c_str();
             }
         }

Release/src/http/client/http_client_winrt.cpp

@@ -28,9 +28,6 @@ using namespace std;
 using namespace Platform;
 using namespace Microsoft::WRL;
 
-#undef min
-#undef max
-
 namespace web
 {
 namespace http
@@ -41,7 +38,7 @@ namespace details
 {
 
 // Additional information necessary to track a WinRT request.
-class winrt_request_context : public request_context
+class winrt_request_context final : public request_context
 {
 public:
 
@@ -63,7 +60,7 @@ class winrt_request_context : public request_context
 };
 
 // Implementation of IXMLHTTPRequest2Callback.
-class HttpRequestCallback :
+class HttpRequestCallback final :
     public RuntimeClass<RuntimeClassFlags<ClassicCom>, IXMLHTTPRequest2Callback, FtmBase>
 {
 public:
@@ -190,7 +187,7 @@ class HttpRequestCallback :
 /// read and write operations. The I/O will be done off the UI thread, so there is no risk
 /// of causing the UI to become unresponsive.
 /// </remarks>
-class IRequestStream
+class IRequestStream final
     : public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<ClassicCom>, ISequentialStream>
 {
 public:
@@ -279,7 +276,7 @@ class IRequestStream
 /// read and write operations. The I/O will be done off the UI thread, so there is no risk
 /// of causing the UI to become unresponsive.
 /// </remarks>
-class IResponseStream
+class IResponseStream final
     : public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<ClassicCom>, ISequentialStream>
 {
 public:
@@ -353,7 +350,7 @@ class IResponseStream
 };
 
 // WinRT client.
-class winrt_client : public _http_client_communicator
+class winrt_client final : public _http_client_communicator
 {
 public:
     winrt_client(http::uri&& address, http_client_config&& client_config)
@@ -379,7 +376,7 @@ class winrt_client : public _http_client_communicator
 protected:
 
     // Start sending request.
-    void send_request(_In_ const std::shared_ptr<request_context> &request)
+    virtual void send_request(_In_ const std::shared_ptr<request_context> &request) override
     {
         http_request &msg = request->m_request;
         auto winrt_context = std::static_pointer_cast<winrt_request_context>(request);

Release/src/http/common/http_msg.cpp

@@ -13,9 +13,6 @@
 #include "stdafx.h"
 #include "../common/internal_http_helpers.h"
 
-#undef min
-#undef max
-
 using namespace web;
 using namespace utility;
 using namespace concurrency;

Release/src/http/listener/http_server_asio.cpp

@@ -14,9 +14,6 @@
 */
 #include "stdafx.h"
 
-#undef min
-#undef max
-
 #include <boost/algorithm/string/find.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/asio/read_until.hpp>
@@ -1085,7 +1082,7 @@ will_deref_and_erase_t asio_server_connection::cancel_sending_response_with_erro
 {
     auto * context = static_cast<linux_request_context*>(response._get_server_context());
     context->m_response_completed.set_exception(eptr);
-    
+
     // always terminate the connection since error happens
     return finish_request_response();
 }
@@ -1096,7 +1093,7 @@ will_deref_and_erase_t asio_server_connection::handle_write_chunked_response(con
     {
         return handle_response_written(response, ec);
     }
-        
+
     auto readbuf = response._get_impl()->instream().streambuf();
     if (readbuf.is_eof())
     {