Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: cache date string used for setting date header #221

Merged
merged 1 commit into from
Nov 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions source/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ add_library(
http/codes.cc
http/conn_manager_impl.cc
http/conn_manager_utility.cc
http/date_provider_impl.cc
http/header_map_impl.cc
http/message_impl.cc
http/pooled_stream_encoder.cc
Expand Down
3 changes: 1 addition & 2 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ void ConnectionManagerImpl::onDrainTimeout() {
checkForDeferredClose();
}

DateFormatter ConnectionManagerImpl::ActiveStream::date_formatter_("%a, %d %b %Y %H:%M:%S GMT");
std::atomic<uint64_t> ConnectionManagerImpl::ActiveStream::next_stream_id_(0);

ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connection_manager)
Expand Down Expand Up @@ -527,8 +526,8 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte
}

// Base headers.
connection_manager_.config_.dateProvider().setDateHeader(headers);
headers.insertServer().value(connection_manager_.config_.serverName());
headers.insertDate().value(date_formatter_.now());
headers.insertEnvoyProtocolVersion().value(connection_manager_.codec_->protocolString());
ConnectionManagerUtility::mutateResponseHeaders(headers, *request_headers_,
connection_manager_.config_);
Expand Down
9 changes: 6 additions & 3 deletions source/common/http/conn_manager_impl.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "date_provider.h"
#include "user_agent.h"

#include "envoy/event/deferred_deletable.h"
Expand All @@ -15,7 +16,6 @@
#include "envoy/upstream/upstream.h"

#include "common/common/linked_object.h"
#include "common/common/utility.h"
#include "common/http/access_log/request_info_impl.h"

namespace Http {
Expand Down Expand Up @@ -118,6 +118,11 @@ class ConnectionManagerConfig {
const Buffer::Instance& data,
ServerConnectionCallbacks& callbacks) PURE;

/**
* @return DateProvider& the date provider to use for
*/
virtual DateProvider& dateProvider() PURE;

/**
* @return the time in milliseconds the connection manager will wait betwen issuing a "shutdown
* notice" to the time it will issue a full GOAWAY and not accept any new streams.
Expand Down Expand Up @@ -370,8 +375,6 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
// Tracing::TracingContext
virtual const std::string& operationName() const override;

static DateFormatter date_formatter_;

// All state for the stream. Put here for readability. We could move this to a bit field
// eventually if we want.
struct State {
Expand Down
22 changes: 22 additions & 0 deletions source/common/http/date_provider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#pragma once

#include "envoy/common/pure.h"
#include "envoy/http/header_map.h"

namespace Http {

/**
* Fills headers with a date header.
*/
class DateProvider {
public:
virtual ~DateProvider() {}

/**
* Set the Date header potentially using a cached value.
* @param headers supplies the headers to fill.
*/
virtual void setDateHeader(HeaderMap& headers) PURE;
};

} // Http
32 changes: 32 additions & 0 deletions source/common/http/date_provider_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#include "date_provider_impl.h"

namespace Http {

DateFormatter DateProviderImplBase::date_formatter_("%a, %d %b %Y %H:%M:%S GMT");

TlsCachingDateProviderImpl::TlsCachingDateProviderImpl(Event::Dispatcher& dispatcher,
ThreadLocal::Instance& tls)
: tls_(tls), tls_slot_(tls.allocateSlot()),
refresh_timer_(dispatcher.createTimer([this]() -> void { onRefreshDate(); })) {

onRefreshDate();
}

void TlsCachingDateProviderImpl::onRefreshDate() {
std::string new_date_string = date_formatter_.now();
tls_.set(tls_slot_, [new_date_string](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectPtr {
return ThreadLocal::ThreadLocalObjectPtr{new ThreadLocalCachedDate(new_date_string)};
});

refresh_timer_->enableTimer(std::chrono::milliseconds(500));
}

void TlsCachingDateProviderImpl::setDateHeader(HeaderMap& headers) {
headers.insertDate().value(tls_.getTyped<ThreadLocalCachedDate>(tls_slot_).date_string_);
}

void SlowDateProviderImpl::setDateHeader(HeaderMap& headers) {
headers.insertDate().value(date_formatter_.now());
}

} // Http
57 changes: 57 additions & 0 deletions source/common/http/date_provider_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#pragma once

#include "date_provider.h"

#include "envoy/event/dispatcher.h"
#include "envoy/thread_local/thread_local.h"

#include "common/common/utility.h"

namespace Http {

/**
* Base for all providers.
*/
class DateProviderImplBase : public DateProvider {
protected:
static DateFormatter date_formatter_;
};

/**
* A caching thread local provider. This implementation updates the date string every 500ms and
* caches on each thread.
*/
class TlsCachingDateProviderImpl : public DateProviderImplBase {
public:
TlsCachingDateProviderImpl(Event::Dispatcher& dispatcher, ThreadLocal::Instance& tls);

// Http::DateProvider
void setDateHeader(HeaderMap& headers) override;

private:
struct ThreadLocalCachedDate : public ThreadLocal::ThreadLocalObject {
ThreadLocalCachedDate(const std::string& date_string) : date_string_(date_string) {}

// ThreadLocal::ThreadLocalObject
void shutdown() override {}

const std::string date_string_;
};

void onRefreshDate();

ThreadLocal::Instance& tls_;
uint32_t tls_slot_;
Event::TimerPtr refresh_timer_;
};

/**
* A basic provider that just creates the date string every time.
*/
class SlowDateProviderImpl : public DateProviderImplBase {
public:
// Http::DateProvider
void setDateHeader(HeaderMap& headers) override;
};

} // Http
3 changes: 2 additions & 1 deletion source/server/config/network/http_connection_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(const Json::Object& con
route_config_(new Router::ConfigImpl(config.getObject("route_config"), server.runtime(),
server.clusterManager())),
drain_timeout_(config.getInteger("drain_timeout_ms", 5000)),
generate_request_id_(config.getBoolean("generate_request_id", true)) {
generate_request_id_(config.getBoolean("generate_request_id", true)),
date_provider_(server.dispatcher(), server.threadLocal()) {

if (config.hasObject("use_remote_address")) {
use_remote_address_ = config.getBoolean("use_remote_address");
Expand Down
3 changes: 3 additions & 0 deletions source/server/config/network/http_connection_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "common/common/logger.h"
#include "common/http/conn_manager_impl.h"
#include "common/http/date_provider_impl.h"
#include "common/json/json_loader.h"

namespace Server {
Expand Down Expand Up @@ -62,6 +63,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
Http::ServerConnectionPtr createCodec(Network::Connection& connection,
const Buffer::Instance& data,
Http::ServerConnectionCallbacks& callbacks) override;
Http::DateProvider& dateProvider() override { return date_provider_; }
std::chrono::milliseconds drainTimeout() override { return drain_timeout_; }
FilterChainFactory& filterFactory() override { return *this; }
bool generateRequestId() override { return generate_request_id_; }
Expand Down Expand Up @@ -107,6 +109,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
Router::ConfigPtr route_config_;
std::chrono::milliseconds drain_timeout_;
bool generate_request_id_;
Http::TlsCachingDateProviderImpl date_provider_;
};

/**
Expand Down
3 changes: 3 additions & 0 deletions source/server/http/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "common/common/logger.h"
#include "common/http/conn_manager_impl.h"
#include "common/http/date_provider_impl.h"
#include "common/http/utility.h"
#include "server/config/network/http_connection_manager.h"

Expand Down Expand Up @@ -43,6 +44,7 @@ class AdminImpl : public Admin,
Http::ServerConnectionPtr createCodec(Network::Connection& connection,
const Buffer::Instance& data,
Http::ServerConnectionCallbacks& callbacks) override;
Http::DateProvider& dateProvider() override { return date_provider_; }
std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); }
Http::FilterChainFactory& filterFactory() override { return *this; }
bool generateRequestId() override { return false; }
Expand Down Expand Up @@ -102,6 +104,7 @@ class AdminImpl : public Admin,
Optional<std::chrono::milliseconds> idle_timeout_;
Optional<std::string> user_agent_;
Optional<Http::TracingConnectionManagerConfig> tracing_config_;
Http::SlowDateProviderImpl date_provider_;
};

/**
Expand Down
1 change: 1 addition & 0 deletions source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include "common/access_log/access_log_manager.h"
#include "common/api/api_impl.h"
#include "common/common/utility.h"
#include "common/common/version.h"
#include "common/memory/stats.h"
#include "common/network/utility.h"
Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ add_executable(envoy-test
common/http/common.cc
common/http/conn_manager_impl_test.cc
common/http/conn_manager_utility_test.cc
common/http/date_provider_impl_test.cc
common/http/filter/buffer_filter_test.cc
common/http/filter/ratelimit_test.cc
common/http/header_map_impl_test.cc
Expand Down
1 change: 1 addition & 0 deletions test/common/http/access_log/access_log_formatter_test.cc
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "common/common/utility.h"
#include "common/http/access_log/access_log_formatter.h"
#include "common/http/header_map_impl.h"

Expand Down
3 changes: 3 additions & 0 deletions test/common/http/conn_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "common/http/access_log/access_log_impl.h"
#include "common/http/access_log/access_log_formatter.h"
#include "common/http/conn_manager_impl.h"
#include "common/http/date_provider_impl.h"
#include "common/http/exception.h"
#include "common/http/headers.h"
#include "common/http/header_map_impl.h"
Expand Down Expand Up @@ -69,6 +70,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi
ServerConnectionCallbacks&) override {
return ServerConnectionPtr{codec_};
}
Http::DateProvider& dateProvider() override { return date_provider_; }
std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); }
FilterChainFactory& filterFactory() override { return filter_factory_; }
bool generateRequestId() override { return true; }
Expand Down Expand Up @@ -106,6 +108,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi
std::unique_ptr<Ssl::MockConnection> ssl_connection_;
NiceMock<Router::MockConfig> route_config_;
Optional<Http::TracingConnectionManagerConfig> tracing_config_;
Http::SlowDateProviderImpl date_provider_;
};

TEST_F(HttpConnectionManagerImplTest, HeaderOnlyRequestAndResponse) {
Expand Down
30 changes: 30 additions & 0 deletions test/common/http/date_provider_impl_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#include "common/http/date_provider_impl.h"
#include "common/http/header_map_impl.h"

#include "test/mocks/event/mocks.h"
#include "test/mocks/thread_local/mocks.h"

using testing::NiceMock;

namespace Http {

TEST(DateProviderImplTest, All) {
Event::MockDispatcher dispatcher;
NiceMock<ThreadLocal::MockInstance> tls;
Event::MockTimer* timer = new Event::MockTimer(&dispatcher);
EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(500)));

TlsCachingDateProviderImpl provider(dispatcher, tls);
HeaderMapImpl headers;
provider.setDateHeader(headers);
EXPECT_NE(nullptr, headers.Date());

EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(500)));
timer->callback_();

headers.removeDate();
provider.setDateHeader(headers);
EXPECT_NE(nullptr, headers.Date());
}

} // Http
1 change: 1 addition & 0 deletions test/mocks/http/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig {
MOCK_METHOD0(accessLogs, const std::list<AccessLog::InstancePtr>&());
MOCK_METHOD3(createCodec_, ServerConnection*(Network::Connection&, const Buffer::Instance&,
ServerConnectionCallbacks&));
MOCK_METHOD0(dateProvider, DateProvider&());
MOCK_METHOD0(drainTimeout, std::chrono::milliseconds());
MOCK_METHOD0(filterFactory, FilterChainFactory&());
MOCK_METHOD0(generateRequestId, bool());
Expand Down