Various micro perf fixes and cleanup found while implementing CN overrides so far#818
Various micro perf fixes and cleanup found while implementing CN overrides so far#818ras0219-msft merged 16 commits intomicrosoft:masterfrom BillyONeal:perf_fixes
Conversation
| lock_guard<mutex> lock(m_action_lock); | ||
| //std::cout << "on_open" << std::endl; | ||
| m_actions.push(action(SUBSCRIBE,hdl)); | ||
| lock.unlock(); |
There was a problem hiding this comment.
Hmmm apparently this isn't built?
| typedef char char_t; | ||
| typedef std::string string_t; | ||
| #define _XPLATSTR(x) x | ||
| #define _XPLATTOSTR(x) std::to_string(x) |
There was a problem hiding this comment.
Perhaps it makes more sense to define a template<class T> string_t to_string_t(T&&)?
|
|
||
| using websocketpp::lib::thread; | ||
| using websocketpp::lib::mutex; | ||
| using websocketpp::lib::lock_guard; |
There was a problem hiding this comment.
This is part of the vendored copy of websocketpp, so we should avoid modifying it.
There was a problem hiding this comment.
I see, I'll revert changes in this example.
| #endif | ||
|
|
||
| #define CRLF std::string("\r\n") | ||
| const std::string CRLF("\r\n"); |
There was a problem hiding this comment.
This should probably be static or moved into a details namespace to avoid conflicting symbols if the user static links.
| 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"; |
There was a problem hiding this comment.
May I ask what is the reason for using both CRLF and hardcoded "\r\n" in this code snippet?
There was a problem hiding this comment.
When CRLF was the entire constant, I left it alone; worst case it saves a strlen on the crlf. Where CRLF was causing an additional trip through <<, I removed it.
There was a problem hiding this comment.
I understand, thx for reply.
# Conflicts: # Release/src/http/client/http_client_asio.cpp
# Conflicts: # Release/src/http/client/http_client_asio.cpp # Release/src/http/client/http_client_winrt.cpp
|
@BillyONeal Thanks! @garethsb-sony thanks for also reviewing :) |
4 participants
No description provided.