Skip to content

Commit

Permalink
server: Move a few internal APIs from std::string to absl::string_vie…
Browse files Browse the repository at this point in the history
…w. (#2889)

Description: Changes a few internal interfaces from std::string to absl::string_view, reducing the number of temporary allocations and memcpys that must be made.

Risk Level: Low
Testing: //test/..., tsan, asan, msan (sanitizers in progress)
Release Notes: N/A

Signed-off-by: Joshua Marantz <jmarantz@google.com>
  • Loading branch information
jmarantz authored and htuch committed Mar 25, 2018
1 parent 5123d31 commit 833769c
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 67 deletions.
8 changes: 6 additions & 2 deletions include/envoy/http/header_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#include "envoy/common/pure.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Http {

Expand Down Expand Up @@ -87,9 +89,11 @@ class HeaderString {
const char* c_str() const { return buffer_.ref_; }

/**
* @return a std::string.
* @return an absl::string_view.
*/
std::string getString() const { return std::string(buffer_.ref_, string_length_); }
absl::string_view getStringView() const {
return absl::string_view(buffer_.ref_, string_length_);
}

/**
* Return the string to a default state. Reference strings are not touched. Both inline/dynamic
Expand Down
12 changes: 8 additions & 4 deletions include/envoy/server/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "envoy/http/header_map.h"
#include "envoy/network/listen_socket.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Server {

Expand All @@ -19,8 +21,10 @@ namespace Server {
* done in the RouteConfigProviderManagerImpl constructor in source/common/router/rds_impl.cc.
*/
#define MAKE_ADMIN_HANDLER(X) \
[this](const std::string& url, Http::HeaderMap& response_headers, \
Buffer::Instance& data) -> Http::Code { return X(url, response_headers, data); }
[this](absl::string_view path_and_query, Http::HeaderMap& response_headers, \
Buffer::Instance& data) -> Http::Code { \
return X(path_and_query, response_headers, data); \
}

/**
* Global admin HTTP endpoint for the server.
Expand All @@ -37,8 +41,8 @@ class Admin {
* @param response supplies the buffer to fill in with the response body.
* @return Http::Code the response code.
*/
typedef std::function<Http::Code(const std::string& url, Http::HeaderMap& response_headers,
Buffer::Instance& response)>
typedef std::function<Http::Code(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response)>
HandlerCb;

/**
Expand Down
4 changes: 2 additions & 2 deletions source/common/common/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ std::string StringUtil::join(const std::vector<std::string>& source, const std::
return ret.substr(0, ret.length() - delimiter.length());
}

std::string StringUtil::subspan(const std::string& source, size_t start, size_t end) {
return source.substr(start, end - start);
std::string StringUtil::subspan(absl::string_view source, size_t start, size_t end) {
return std::string(source.data() + start, end - start);
}

std::string StringUtil::escape(const std::string& source) {
Expand Down
3 changes: 2 additions & 1 deletion source/common/common/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,9 @@ class StringUtil {
/**
* Version of substr() that operates on a start and end index instead of a start index and a
* length.
* @return string substring starting at start, and ending right before end.
*/
static std::string subspan(const std::string& source, size_t start, size_t end);
static std::string subspan(absl::string_view source, size_t start, size_t end);

/**
* Escape strings for logging purposes. Returns a copy of the string with
Expand Down
7 changes: 5 additions & 2 deletions source/common/ext_authz/ext_authz_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ void CheckRequestUtils::setAttrContextPeer(envoy::service::auth::v2::AttributeCo

std::string CheckRequestUtils::getHeaderStr(const Envoy::Http::HeaderEntry* entry) {
if (entry) {
return entry->value().getString();
// TODO(jmarantz): plumb absl::string_view further here; there's no need
// to allocate a temp string in the local uses.
return std::string(entry->value().getStringView());
}
return "";
}
Expand Down Expand Up @@ -148,7 +150,8 @@ void CheckRequestUtils::setHttpRequest(
mutable_headers = static_cast<
Envoy::Protobuf::Map<Envoy::ProtobufTypes::String, Envoy::ProtobufTypes::String>*>(
ctx);
(*mutable_headers)[e.key().getString()] = e.value().getString();
(*mutable_headers)[std::string(e.key().getStringView())] =
std::string(e.value().getStringView());
return Envoy::Http::HeaderMap::Iterate::Continue;
},
mutable_headers);
Expand Down
4 changes: 2 additions & 2 deletions source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ std::string Utility::createSslRedirectPath(const HeaderMap& headers) {
headers.Path()->value().c_str());
}

Utility::QueryParams Utility::parseQueryString(const std::string& url) {
Utility::QueryParams Utility::parseQueryString(absl::string_view url) {
QueryParams params;
size_t start = url.find('?');
if (start == std::string::npos) {
Expand All @@ -57,7 +57,7 @@ Utility::QueryParams Utility::parseQueryString(const std::string& url) {
if (end == std::string::npos) {
end = url.size();
}
absl::string_view param(url.c_str() + start, end - start);
absl::string_view param(url.data() + start, end - start);

const size_t equal = param.find('=');
if (equal != std::string::npos) {
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Utility {
* @param url supplies the url to parse.
* @return QueryParams the parsed parameters, if any.
*/
static QueryParams parseQueryString(const std::string& url);
static QueryParams parseQueryString(absl::string_view url);

/**
* Finds the start of the query string in a path
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/rds_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::getRouteCon
return new_provider;
};

Http::Code RouteConfigProviderManagerImpl::handlerRoutes(const std::string& url, Http::HeaderMap&,
Http::Code RouteConfigProviderManagerImpl::handlerRoutes(absl::string_view url, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url);
// If there are no query params, print out all the configured route tables.
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/rds_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ class RouteConfigProviderManagerImpl : public ServerRouteConfigProviderManager,
* @param response supplies the buffer to fill with information.
* @return Http::Code OK if the endpoint can parse and operate on the url, NotFound otherwise.
*/
Http::Code handlerRoutes(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerRoutes(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);

/**
Expand Down
5 changes: 2 additions & 3 deletions source/common/tracing/zipkin/zipkin_tracer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, Http::HeaderMa
if (request_headers.XB3Sampled()) {
// Checking if sampled flag has been specified. Also checking for 'true' value, as some old
// zipkin tracers may still use that value, although should be 0 or 1.
sampled =
request_headers.XB3Sampled()->value().getString() == ZipkinCoreConstants::get().SAMPLED ||
request_headers.XB3Sampled()->value().getString() == "true";
absl::string_view xb3_sampled = request_headers.XB3Sampled()->value().getStringView();
sampled = xb3_sampled == ZipkinCoreConstants::get().SAMPLED || xb3_sampled == "true";
}

if (request_headers.XB3TraceId() && request_headers.XB3SpanId()) {
Expand Down
38 changes: 19 additions & 19 deletions source/server/http/admin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "common/upstream/host_utility.h"

#include "absl/strings/str_replace.h"
#include "absl/strings/string_view.h"

// TODO(mattklein123): Switch to JSON interface methods and remove rapidjson dependency.
#include "rapidjson/document.h"
Expand Down Expand Up @@ -218,7 +219,7 @@ void AdminImpl::addCircuitSettings(const std::string& cluster_name, const std::s
resource_manager.retries().max()));
}

Http::Code AdminImpl::handlerClusters(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerClusters(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
response.add(fmt::format("version_info::{}\n", server_.clusterManager().versionInfo()));

Expand Down Expand Up @@ -275,7 +276,7 @@ Http::Code AdminImpl::handlerClusters(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerCpuProfiler(const std::string& url, Http::HeaderMap&,
Http::Code AdminImpl::handlerCpuProfiler(absl::string_view url, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url);
if (query_params.size() != 1 || query_params.begin()->first != "enable" ||
Expand All @@ -299,27 +300,27 @@ Http::Code AdminImpl::handlerCpuProfiler(const std::string& url, Http::HeaderMap
return Http::Code::OK;
}

Http::Code AdminImpl::handlerHealthcheckFail(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerHealthcheckFail(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
server_.failHealthcheck(true);
response.add("OK\n");
return Http::Code::OK;
}

Http::Code AdminImpl::handlerHealthcheckOk(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerHealthcheckOk(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
server_.failHealthcheck(false);
response.add("OK\n");
return Http::Code::OK;
}

Http::Code AdminImpl::handlerHotRestartVersion(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerHotRestartVersion(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
response.add(server_.hotRestart().version());
return Http::Code::OK;
}

Http::Code AdminImpl::handlerLogging(const std::string& url, Http::HeaderMap&,
Http::Code AdminImpl::handlerLogging(absl::string_view url, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url);

Expand All @@ -345,7 +346,7 @@ Http::Code AdminImpl::handlerLogging(const std::string& url, Http::HeaderMap&,
return rc;
}

Http::Code AdminImpl::handlerResetCounters(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerResetCounters(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
for (const Stats::CounterSharedPtr& counter : server_.stats().counters()) {
counter->reset();
Expand All @@ -355,7 +356,7 @@ Http::Code AdminImpl::handlerResetCounters(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerServerInfo(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerServerInfo(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
time_t current_time = time(nullptr);
response.add(fmt::format("envoy {} {} {} {} {}\n", VersionInfo::version(),
Expand All @@ -366,7 +367,7 @@ Http::Code AdminImpl::handlerServerInfo(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerStats(const std::string& url, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerStats(absl::string_view url, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
// We currently don't support timers locally (only via statsd) so just group all the counters
// and gauges together, alpha sort them, and spit them out.
Expand Down Expand Up @@ -477,14 +478,14 @@ std::string AdminImpl::statsAsJson(const std::map<std::string, uint64_t>& all_st
return strbuf.GetString();
}

Http::Code AdminImpl::handlerQuitQuitQuit(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerQuitQuitQuit(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
server_.shutdown();
response.add("OK\n");
return Http::Code::OK;
}

Http::Code AdminImpl::handlerListenerInfo(const std::string&, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerListenerInfo(absl::string_view, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
response_headers.insertContentType().value().setReference(
Http::Headers::get().ContentTypeValues.Json);
Expand All @@ -496,7 +497,7 @@ Http::Code AdminImpl::handlerListenerInfo(const std::string&, Http::HeaderMap& r
return Http::Code::OK;
}

Http::Code AdminImpl::handlerCerts(const std::string&, Http::HeaderMap&,
Http::Code AdminImpl::handlerCerts(absl::string_view, Http::HeaderMap&,
Buffer::Instance& response) {
// This set is used to track distinct certificates. We may have multiple listeners, upstreams, etc
// using the same cert.
Expand All @@ -515,7 +516,7 @@ Http::Code AdminImpl::handlerCerts(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerRuntime(const std::string& url, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerRuntime(absl::string_view url, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
Http::Code rc = Http::Code::OK;
const Http::Utility::QueryParams params = Http::Utility::parseQueryString(url);
Expand Down Expand Up @@ -585,7 +586,7 @@ std::string AdminImpl::runtimeAsJson(
}

void AdminFilter::onComplete() {
std::string path = request_headers_->Path()->value().c_str();
absl::string_view path = request_headers_->Path()->value().getStringView();
ENVOY_STREAM_LOG(debug, "request complete: path: {}", *callbacks_, path);

Buffer::OwnedImpl response;
Expand Down Expand Up @@ -686,7 +687,7 @@ void AdminImpl::createFilterChain(Http::FilterChainFactoryCallbacks& callbacks)
callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{new AdminFilter(*this)});
}

Http::Code AdminImpl::runCallback(const std::string& path_and_query,
Http::Code AdminImpl::runCallback(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response) {
Http::Code code = Http::Code::OK;
bool found_handler = false;
Expand Down Expand Up @@ -722,12 +723,11 @@ std::vector<const AdminImpl::UrlHandler*> AdminImpl::sortedHandlers() const {
}
// Note: it's generally faster to sort a vector with std::vector than to construct a std::map.
std::sort(sorted_handlers.begin(), sorted_handlers.end(),
[&](const UrlHandler* h1, const UrlHandler* h2) { return h1->prefix_ < h2->prefix_; });
[](const UrlHandler* h1, const UrlHandler* h2) { return h1->prefix_ < h2->prefix_; });
return sorted_handlers;
}

Http::Code AdminImpl::handlerHelp(const std::string&, Http::HeaderMap&,
Buffer::Instance& response) {
Http::Code AdminImpl::handlerHelp(absl::string_view, Http::HeaderMap&, Buffer::Instance& response) {
response.add("admin commands are:\n");

// Prefix order is used during searching, but for printing do them in alpha order.
Expand All @@ -737,7 +737,7 @@ Http::Code AdminImpl::handlerHelp(const std::string&, Http::HeaderMap&,
return Http::Code::OK;
}

Http::Code AdminImpl::handlerAdminHome(const std::string&, Http::HeaderMap& response_headers,
Http::Code AdminImpl::handlerAdminHome(absl::string_view, Http::HeaderMap& response_headers,
Buffer::Instance& response) {
response_headers.insertContentType().value().setReference(
Http::Headers::get().ContentTypeValues.Html);
Expand Down
36 changes: 19 additions & 17 deletions source/server/http/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include "server/config/network/http_connection_manager.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Server {

Expand All @@ -41,7 +43,7 @@ class AdminImpl : public Admin,
const std::string& address_out_path, Network::Address::InstanceConstSharedPtr address,
Server::Instance& server, Stats::ScopePtr&& listener_scope);

Http::Code runCallback(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code runCallback(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
const Network::Socket& socket() override { return *socket_; }
Network::Socket& mutable_socket() { return *socket_; }
Expand Down Expand Up @@ -136,37 +138,37 @@ class AdminImpl : public Admin,
/**
* URL handlers.
*/
Http::Code handlerAdminHome(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerAdminHome(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerCerts(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerCerts(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerClusters(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerClusters(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerCpuProfiler(const std::string& path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerHealthcheckFail(const std::string& path_and_query,
Http::Code handlerCpuProfiler(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerHealthcheckFail(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerHealthcheckOk(const std::string& path_and_query,
Http::Code handlerHealthcheckOk(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerHelp(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerHelp(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerHotRestartVersion(const std::string& path_and_query,
Http::Code handlerHotRestartVersion(absl::string_view path_and_query,
Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerListenerInfo(const std::string& path_and_query,
Http::Code handlerListenerInfo(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerLogging(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerLogging(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerMain(const std::string& path, Buffer::Instance& response);
Http::Code handlerQuitQuitQuit(const std::string& path_and_query,
Http::Code handlerQuitQuitQuit(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerResetCounters(const std::string& path_and_query,
Http::Code handlerResetCounters(absl::string_view path_and_query,
Http::HeaderMap& response_headers, Buffer::Instance& response);
Http::Code handlerServerInfo(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerServerInfo(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerStats(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerStats(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);
Http::Code handlerRuntime(const std::string& path_and_query, Http::HeaderMap& response_headers,
Http::Code handlerRuntime(absl::string_view path_and_query, Http::HeaderMap& response_headers,
Buffer::Instance& response);

class AdminListener : public Network::ListenerConfig {
Expand Down
Loading

0 comments on commit 833769c

Please sign in to comment.