From 829fef34a6abf162e3c7c2a7dea681f359507b61 Mon Sep 17 00:00:00 2001 From: Marco Magdy Date: Wed, 24 Nov 2021 11:36:04 -0800 Subject: [PATCH 1/2] Reduce string copying. Optimized a few unnecessary string copies that should have been moves to begin with. Additionally, I noticed that we're scanning the headers twice once to test if a given header exists and another to get its value, that is a perfect use case for std::optional, but since we need to stay compatible with C++11 and C++14, we can use the 'outcome' class instead. --- include/aws/http/response.h | 13 +++++--- include/aws/lambda-runtime/outcome.h | 23 ++++++++++++--- src/runtime.cpp | 44 ++++++++++++++++------------ 3 files changed, 53 insertions(+), 27 deletions(-) diff --git a/include/aws/http/response.h b/include/aws/http/response.h index 9b8cbda..81f5071 100644 --- a/include/aws/http/response.h +++ b/include/aws/http/response.h @@ -14,6 +14,8 @@ * permissions and limitations under the License. */ +#include "aws/lambda-runtime/outcome.h" + #include #include #include @@ -31,7 +33,7 @@ class response { inline void add_header(std::string name, std::string const& value); inline void append_body(const char* p, size_t sz); inline bool has_header(char const* header) const; - inline std::string const& get_header(char const* header) const; + inline lambda_runtime::outcome get_header(char const* header) const; inline response_code get_response_code() const { return m_response_code; } inline void set_response_code(aws::http::response_code c); inline void set_content_type(char const* ct); @@ -140,7 +142,7 @@ inline std::string const& response::get_body() const inline void response::add_header(std::string name, std::string const& value) { std::transform(name.begin(), name.end(), name.begin(), ::tolower); - m_headers.emplace_back(name, value); + m_headers.emplace_back(std::move(name), value); } inline void response::append_body(const char* p, size_t sz) @@ -161,12 +163,15 @@ inline bool response::has_header(char const* header) const }); } -inline std::string const& response::get_header(char const* header) const +inline lambda_runtime::outcome response::get_header(char const* header) const { auto it = std::find_if(m_headers.begin(), m_headers.end(), [header](std::pair const& p) { return p.first == header; }); - assert(it != m_headers.end()); + + if (it == m_headers.end()) { + return false; + } return it->second; } diff --git a/include/aws/lambda-runtime/outcome.h b/include/aws/lambda-runtime/outcome.h index b5d0b8b..828bc91 100644 --- a/include/aws/lambda-runtime/outcome.h +++ b/include/aws/lambda-runtime/outcome.h @@ -49,14 +49,19 @@ class outcome { } } - ~outcome() + ~outcome() { destroy(); } + + outcome& operator=(outcome&& other) noexcept { - if (m_success) { - m_s.~TResult(); + assert(this != &other); + destroy(); + if (other.m_success) { + new (&m_s) TResult(std::move(other.m_s)); } else { - m_f.~TFailure(); + new (&m_f) TFailure(std::move(other.m_f)); } + return *this; } TResult const& get_result() const& @@ -86,6 +91,16 @@ class outcome { bool is_success() const { return m_success; } private: + void destroy() + { + if (m_success) { + m_s.~TResult(); + } + else { + m_f.~TFailure(); + } + } + union { TResult m_s; TFailure m_f; diff --git a/src/runtime.cpp b/src/runtime.cpp index 9175084..61f9c0a 100644 --- a/src/runtime.cpp +++ b/src/runtime.cpp @@ -116,9 +116,7 @@ static size_t write_header(char* ptr, size_t size, size_t nmemb, void* userdata) if (ptr[i] != ':') { continue; } - std::string key{ptr, i}; - std::string value{ptr + i + 1, nmemb - i - 1}; - resp->add_header(trim(key), trim(value)); + resp->add_header(trim({ptr, i}), trim({ptr + i + 1, nmemb - i - 1})); break; } return size * nmemb; @@ -160,9 +158,11 @@ static int rt_curl_debug_callback(CURL* handle, curl_infotype type, char* data, runtime::runtime(std::string const& endpoint) : runtime(endpoint, "AWS_Lambda_Cpp/" + std::string(get_version())) {} runtime::runtime(std::string const& endpoint, std::string const& user_agent) - : m_user_agent_header("User-Agent: " + user_agent), m_endpoints{{endpoint + "/2018-06-01/runtime/init/error", - endpoint + "/2018-06-01/runtime/invocation/next", - endpoint + "/2018-06-01/runtime/invocation/"}}, + : m_user_agent_header("User-Agent: " + user_agent), + m_endpoints{ + {endpoint + "/2018-06-01/runtime/init/error", + endpoint + "/2018-06-01/runtime/invocation/next", + endpoint + "/2018-06-01/runtime/invocation/"}}, m_curl_handle(curl_easy_init()) { if (!m_curl_handle) { @@ -264,32 +264,38 @@ runtime::next_outcome runtime::get_next() return resp.get_response_code(); } - if (!resp.has_header(REQUEST_ID_HEADER)) { + auto out = resp.get_header(REQUEST_ID_HEADER); + if (!out.is_success()) { logging::log_error(LOG_TAG, "Failed to find header %s in response", REQUEST_ID_HEADER); return aws::http::response_code::REQUEST_NOT_MADE; } invocation_request req; req.payload = resp.get_body(); - req.request_id = resp.get_header(REQUEST_ID_HEADER); + req.request_id = std::move(out).get_result(); - if (resp.has_header(TRACE_ID_HEADER)) { - req.xray_trace_id = resp.get_header(TRACE_ID_HEADER); + out = resp.get_header(TRACE_ID_HEADER); + if (out.is_success()) { + req.xray_trace_id = std::move(out).get_result(); } - if (resp.has_header(CLIENT_CONTEXT_HEADER)) { - req.client_context = resp.get_header(CLIENT_CONTEXT_HEADER); + out = resp.get_header(CLIENT_CONTEXT_HEADER); + if (out.is_success()) { + req.client_context = std::move(out).get_result(); } - if (resp.has_header(COGNITO_IDENTITY_HEADER)) { - req.cognito_identity = resp.get_header(COGNITO_IDENTITY_HEADER); + out = resp.get_header(COGNITO_IDENTITY_HEADER); + if (out.is_success()) { + req.cognito_identity = std::move(out).get_result(); } - if (resp.has_header(FUNCTION_ARN_HEADER)) { - req.function_arn = resp.get_header(FUNCTION_ARN_HEADER); + out = resp.get_header(FUNCTION_ARN_HEADER); + if (out.is_success()) { + req.function_arn = std::move(out).get_result(); } - if (resp.has_header(DEADLINE_MS_HEADER)) { - auto const& deadline_string = resp.get_header(DEADLINE_MS_HEADER); + out = resp.get_header(DEADLINE_MS_HEADER); + if (out.is_success()) { + auto const& deadline_string = std::move(out).get_result(); constexpr int base = 10; unsigned long ms = strtoul(deadline_string.c_str(), nullptr, base); assert(ms > 0); @@ -301,7 +307,7 @@ runtime::next_outcome runtime::get_next() req.payload.c_str(), static_cast(req.get_time_remaining().count())); } - return next_outcome(req); + return {req}; } runtime::post_outcome runtime::post_success(std::string const& request_id, invocation_response const& handler_response) From e453524a46b24e628ac50fb9029baab93d74a7ac Mon Sep 17 00:00:00 2001 From: Marco Magdy Date: Mon, 29 Nov 2021 13:28:57 -0800 Subject: [PATCH 2/2] Fixup: Move state boolean in move assignment --- include/aws/lambda-runtime/outcome.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/aws/lambda-runtime/outcome.h b/include/aws/lambda-runtime/outcome.h index 828bc91..9982f5d 100644 --- a/include/aws/lambda-runtime/outcome.h +++ b/include/aws/lambda-runtime/outcome.h @@ -61,6 +61,7 @@ class outcome { else { new (&m_f) TFailure(std::move(other.m_f)); } + m_success = other.m_success; return *this; }