Cherrypick callback manager improvement fix.

source/common/common/callback_impl.h

@@ -24,6 +24,9 @@ template <typename... CallbackArgs> class CallbackManager {
  */
  CallbackHandle* add(Callback callback) {
  callbacks_.emplace_back(*this, callback);
+ // get the list iterator of added callback handle, which will be used to remove itself from
+ // callbacks_ list.
+ callbacks_.back().it_ = (--callbacks_.end());
  return &callbacks_.back();
  }
 
@@ -46,24 +49,21 @@ template <typename... CallbackArgs> class CallbackManager {
  CallbackHolder(CallbackManager& parent, Callback cb) : parent_(parent), cb_(cb) {}
 
  // CallbackHandle
- void remove() override { parent_.remove(this); }
+ void remove() override { parent_.remove(it_); }
 
  CallbackManager& parent_;
  Callback cb_;
+
+ // the iterator of this callback holder inside callbacks_ list
+ // upon removal, use this iterator to delete callback holder in O(1)
+ typename std::list<CallbackHolder>::iterator it_;
  };
 
  /**
  * Remove a member update callback added via add().
  * @param handle supplies the callback handle to remove.
  */
- void remove(CallbackHandle* handle) {
- ASSERT(std::find_if(callbacks_.begin(), callbacks_.end(),
- [handle](const CallbackHolder& holder) -> bool {
- return handle == &holder;
- }) != callbacks_.end());
- callbacks_.remove_if(
- [handle](const CallbackHolder& holder) -> bool { return handle == &holder; });
- }
+ void remove(typename std::list<CallbackHolder>::iterator& it) { callbacks_.erase(it); }
 
  std::list<CallbackHolder> callbacks_;
 };

test/integration/stats_integration_test.cc

@@ -274,6 +274,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) {
  // 2020/05/13 10531 44425 44600 Refactor resource manager
  // 2020/04/23 10661 44425 44600 per-listener connection limits
  // 2020/06/09 tbd 43541 44600 add stats and stream flush timeout
+ // 2020/06/29 11751 43765 46000 Improve time complexity of removing callback handle
+ // in callback manager.
 
  // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI
  // 'release' builds, where we control the platform and tool-chain. So you
@@ -287,8 +289,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) {
  // If you encounter a failure here, please see
  // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests
  // for details on how to fix.
- EXPECT_MEMORY_EQ(m_per_cluster, 43541);
- EXPECT_MEMORY_LE(m_per_cluster, 44600);
+ EXPECT_MEMORY_EQ(m_per_cluster, 43765);
+ EXPECT_MEMORY_LE(m_per_cluster, 46000); // Round up to allow platform variations.
 }
 
 TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) {
@@ -335,6 +337,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) {
  // 2020/05/13 10531 36537 44600 Refactor resource manager
  // 2020/04/23 10661 36537 36800 per-listener connection limits
  // 2020/06/09 tbd 35749 36800 add stats and stream flush timeout
+ // 2020/06/29 11751 35973 38000 Improve time complexity of removing callback handle.
+ // in callback manager.
 
  // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI
  // 'release' builds, where we control the platform and tool-chain. So you
@@ -348,8 +352,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) {
  // If you encounter a failure here, please see
  // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests
  // for details on how to fix.
- EXPECT_MEMORY_EQ(m_per_cluster, 35749);
- EXPECT_MEMORY_LE(m_per_cluster, 36800);
+ EXPECT_MEMORY_EQ(m_per_cluster, 35973);
+ EXPECT_MEMORY_LE(m_per_cluster, 38000); // Round up to allow platform variations.
 }
 
 TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) {