Skip to content

Commit

Permalink
filter chain: prefer vector over list for filter chain (envoyproxy#37665
Browse files Browse the repository at this point in the history 
)

Commit Message: filter chain: prefer vector over list for filter chain
Additional Description:

Use vector to store the filter chain rather than list. Note, we never
require to insert/remove element in the intermediate node of filter
chain container. In this scenario, vector is much more memory-friend
than list.

Risk Level: mid.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features:  n/a.

---------

Signed-off-by: wangbaiping(wbpcode) <[email protected]>
  • Loading branch information
@wbpcode
wbpcode authored Dec 19, 2024
1 parent b9c4ff2 commit 4fa73a8
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 79 deletions.
58 changes: 33 additions & 25 deletions source/common/http/filter_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ namespace Http {

namespace {

template <class T> using FilterList = std::list<std::unique_ptr<T>>;

// Shared helper for recording the latest filter used.
template <class T>
void recordLatestDataFilter(const typename FilterList<T>::iterator current_filter,
T*& latest_filter, const FilterList<T>& filters) {
template <class Filters>
void recordLatestDataFilter(typename Filters::Iterator current_filter,
typename Filters::Element*& latest_filter, Filters& filters) {
// If this is the first time we're calling onData, just record the current filter.
if (latest_filter == nullptr) {
latest_filter = current_filter->get();
Expand Down Expand Up @@ -520,8 +518,7 @@ void FilterManager::applyFilterFactoryCb(FilterContext context, FilterFactoryCb&
factory(callbacks);
}

void FilterManager::maybeContinueDecoding(
const std::list<ActiveStreamDecoderFilterPtr>::iterator& continue_data_entry) {
void FilterManager::maybeContinueDecoding(StreamDecoderFilters::Iterator continue_data_entry) {
if (continue_data_entry != decoder_filters_.end()) {
// We use the continueDecoding() code since it will correctly handle not calling
// decodeHeaders() again. Fake setting StopSingleIteration since the continueDecoding() code
Expand All @@ -536,9 +533,9 @@ void FilterManager::maybeContinueDecoding(
void FilterManager::decodeHeaders(ActiveStreamDecoderFilter* filter, RequestHeaderMap& headers,
bool end_stream) {
// Headers filter iteration should always start with the next filter if available.
std::list<ActiveStreamDecoderFilterPtr>::iterator entry =
StreamDecoderFilters::Iterator entry =
commonDecodePrefix(filter, FilterIterationStartState::AlwaysStartFromNext);
std::list<ActiveStreamDecoderFilterPtr>::iterator continue_data_entry = decoder_filters_.end();
StreamDecoderFilters::Iterator continue_data_entry = decoder_filters_.end();
bool terminal_filter_decoded_end_stream = false;
ASSERT(!state_.decoder_filter_chain_complete_ || entry == decoder_filters_.end() ||
(*entry)->end_stream_);
Expand Down Expand Up @@ -649,8 +646,7 @@ void FilterManager::decodeData(ActiveStreamDecoderFilter* filter, Buffer::Instan
auto trailers_added_entry = decoder_filters_.end();
const bool trailers_exists_at_start = filter_manager_callbacks_.requestTrailers().has_value();
// Filter iteration may start at the current filter.
std::list<ActiveStreamDecoderFilterPtr>::iterator entry =
commonDecodePrefix(filter, filter_iteration_start_state);
StreamDecoderFilters::Iterator entry = commonDecodePrefix(filter, filter_iteration_start_state);
bool terminal_filter_decoded_end_stream = false;
ASSERT(!state_.decoder_filter_chain_complete_ || entry == decoder_filters_.end() ||
(*entry)->end_stream_);
Expand Down Expand Up @@ -801,7 +797,7 @@ void FilterManager::decodeTrailers(ActiveStreamDecoderFilter* filter, RequestTra
}

// Filter iteration may start at the current filter.
std::list<ActiveStreamDecoderFilterPtr>::iterator entry =
StreamDecoderFilters::Iterator entry =
commonDecodePrefix(filter, FilterIterationStartState::CanStartFromCurrent);
bool terminal_filter_reached = false;
ASSERT(!state_.decoder_filter_chain_complete_ || entry == decoder_filters_.end());
Expand Down Expand Up @@ -845,7 +841,7 @@ void FilterManager::decodeMetadata(ActiveStreamDecoderFilter* filter, MetadataMa
filter_manager_callbacks_.resetIdleTimer();

// Filter iteration may start at the current filter.
std::list<ActiveStreamDecoderFilterPtr>::iterator entry =
StreamDecoderFilters::Iterator entry =
commonDecodePrefix(filter, FilterIterationStartState::CanStartFromCurrent);

ASSERT(!(state_.filter_call_state_ & FilterCallState::DecodeMetadata));
Expand Down Expand Up @@ -897,7 +893,7 @@ void FilterManager::decodeMetadata(ActiveStreamDecoderFilter* filter, MetadataMa

void FilterManager::disarmRequestTimeout() { filter_manager_callbacks_.disarmRequestTimeout(); }

std::list<ActiveStreamEncoderFilterPtr>::iterator
StreamEncoderFilters::Iterator
FilterManager::commonEncodePrefix(ActiveStreamEncoderFilter* filter, bool end_stream,
FilterIterationStartState filter_iteration_start_state) {
// Only do base state setting on the initial call. Subsequent calls for filtering do not touch
Expand Down Expand Up @@ -931,7 +927,7 @@ FilterManager::commonEncodePrefix(ActiveStreamEncoderFilter* filter, bool end_st
return std::next(filter->entry());
}

std::list<ActiveStreamDecoderFilterPtr>::iterator
StreamDecoderFilters::Iterator
FilterManager::commonDecodePrefix(ActiveStreamDecoderFilter* filter,
FilterIterationStartState filter_iteration_start_state) {
if (!filter) {
Expand Down Expand Up @@ -1184,7 +1180,7 @@ void FilterManager::encode1xxHeaders(ActiveStreamEncoderFilter* filter,
// end-stream, and because there are normal headers coming there's no need for
// complex continuation logic.
// 100-continue filter iteration should always start with the next filter if available.
std::list<ActiveStreamEncoderFilterPtr>::iterator entry =
StreamEncoderFilters::Iterator entry =
commonEncodePrefix(filter, false, FilterIterationStartState::AlwaysStartFromNext);
for (; entry != encoder_filters_.end(); entry++) {
ENVOY_EXECUTION_SCOPE(trackedStream(), &(*entry)->filter_context_);
Expand All @@ -1210,8 +1206,7 @@ void FilterManager::encode1xxHeaders(ActiveStreamEncoderFilter* filter,
filter_manager_callbacks_.encode1xxHeaders(headers);
}

void FilterManager::maybeContinueEncoding(
const std::list<ActiveStreamEncoderFilterPtr>::iterator& continue_data_entry) {
void FilterManager::maybeContinueEncoding(StreamEncoderFilters::Iterator continue_data_entry) {
if (continue_data_entry != encoder_filters_.end()) {
// We use the continueEncoding() code since it will correctly handle not calling
// encodeHeaders() again. Fake setting StopSingleIteration since the continueEncoding() code
Expand All @@ -1232,9 +1227,9 @@ void FilterManager::encodeHeaders(ActiveStreamEncoderFilter* filter, ResponseHea
disarmRequestTimeout();

// Headers filter iteration should always start with the next filter if available.
std::list<ActiveStreamEncoderFilterPtr>::iterator entry =
StreamEncoderFilters::Iterator entry =
commonEncodePrefix(filter, end_stream, FilterIterationStartState::AlwaysStartFromNext);
std::list<ActiveStreamEncoderFilterPtr>::iterator continue_data_entry = encoder_filters_.end();
StreamEncoderFilters::Iterator continue_data_entry = encoder_filters_.end();

for (; entry != encoder_filters_.end(); entry++) {
ENVOY_EXECUTION_SCOPE(trackedStream(), &(*entry)->filter_context_);
Expand Down Expand Up @@ -1318,7 +1313,7 @@ void FilterManager::encodeMetadata(ActiveStreamEncoderFilter* filter,
MetadataMapPtr&& metadata_map_ptr) {
filter_manager_callbacks_.resetIdleTimer();

std::list<ActiveStreamEncoderFilterPtr>::iterator entry =
StreamEncoderFilters::Iterator entry =
commonEncodePrefix(filter, false, FilterIterationStartState::CanStartFromCurrent);

for (; entry != encoder_filters_.end(); entry++) {
Expand Down Expand Up @@ -1405,9 +1400,9 @@ void FilterManager::encodeData(ActiveStreamEncoderFilter* filter, Buffer::Instan
filter_manager_callbacks_.resetIdleTimer();

// Filter iteration may start at the current filter.
std::list<ActiveStreamEncoderFilterPtr>::iterator entry =
StreamEncoderFilters::Iterator entry =
commonEncodePrefix(filter, end_stream, filter_iteration_start_state);
auto trailers_added_entry = encoder_filters_.end();
StreamEncoderFilters::Iterator trailers_added_entry = encoder_filters_.end();

const bool trailers_exists_at_start = filter_manager_callbacks_.responseTrailers().has_value();
for (; entry != encoder_filters_.end(); entry++) {
Expand Down Expand Up @@ -1479,7 +1474,7 @@ void FilterManager::encodeTrailers(ActiveStreamEncoderFilter* filter,
filter_manager_callbacks_.resetIdleTimer();

// Filter iteration may start at the current filter.
std::list<ActiveStreamEncoderFilterPtr>::iterator entry =
StreamEncoderFilters::Iterator entry =
commonEncodePrefix(filter, true, FilterIterationStartState::CanStartFromCurrent);
for (; entry != encoder_filters_.end(); entry++) {
ENVOY_EXECUTION_SCOPE(trackedStream(), &(*entry)->filter_context_);
Expand Down Expand Up @@ -1664,6 +1659,19 @@ FilterManager::createFilterChain(const FilterChainFactory& filter_chain_factory)
return state_.create_chain_result_;
}

// After the filter chain creation is completed, the filter chain containers will not be
// modified. So, we can safely complete the initialization of the filter chain now.
Cleanup initialize_filter_chain([this]() {
for (auto iter = decoder_filters_.begin(); iter != decoder_filters_.end(); ++iter) {
(*iter)->entry_ = iter;
}
for (auto iter = encoder_filters_.begin(); iter != encoder_filters_.end(); ++iter) {
(*iter)->entry_ = iter;
}
});

// TODO(wbpcode): reserve memory for filters to avoid frequent reallocation.

OptRef<DownstreamStreamFilterCallbacks> downstream_callbacks =
filter_manager_callbacks_.downstreamCallbacks();

Expand Down Expand Up @@ -1918,7 +1926,7 @@ void FilterManager::resetStream(StreamResetReason reason,
}

bool FilterManager::isTerminalDecoderFilter(const ActiveStreamDecoderFilter& filter) const {
return !decoder_filters_.empty() && decoder_filters_.back().get() == &filter;
return !decoder_filters_.entries_.empty() && decoder_filters_.entries_.back().get() == &filter;
}

void ActiveStreamFilterBase::resetStream(Http::StreamResetReason reset_reason,
Expand Down
Loading

0 comments on commit 4fa73a8

Please sign in to comment.