common inline map registry support

envoy/stream_info/BUILD

@@ -36,6 +36,7 @@ envoy_cc_library(
     deps = [
         "//envoy/config:typed_config_interface",
         "//source/common/common:fmt_lib",
+        "//source/common/common:inline_map_registry",
         "//source/common/common:utility_lib",
         "//source/common/protobuf",
     ],

envoy/stream_info/filter_state.h

@@ -7,6 +7,7 @@
 #include "envoy/config/typed_config.h"
 
 #include "source/common/common/fmt.h"
+#include "source/common/common/inline_map_registry.h"
 #include "source/common/common/utility.h"
 #include "source/common/protobuf/protobuf.h"
 
@@ -16,6 +17,13 @@
 namespace Envoy {
 namespace StreamInfo {
 
+class FilterStateInlineMapScope {
+public:
+  static absl::string_view name() { return "inline_map_scope_filter_state"; }
+};
+
+using InlineKey = InlineMapRegistry::Handle;
+
 class FilterState;
 
 using FilterStateSharedPtr = std::shared_ptr<FilterState>;
@@ -173,6 +181,25 @@ class FilterState {
           LifeSpan life_span = LifeSpan::FilterChain,
           StreamSharingMayImpactPooling stream_sharing = StreamSharingMayImpactPooling::None) PURE;
 
+  /**
+   * @param data_key the handle of the data being set.
+   * @param data an owning pointer to the data to be stored.
+   * @param state_type indicates whether the object is mutable or not.
+   * @param life_span indicates the life span of the object: bound to the filter chain, a
+   * request, or a connection.
+   *
+   * Note that it is an error to call setData() twice with the same
+   * data_name, if the existing object is immutable. Similarly, it is an
+   * error to call setData() with same data_name but different state_types
+   * (mutable and readOnly, or readOnly and mutable) or different life_span.
+   * This is to enforce a single authoritative source for each piece of
+   * data stored in FilterState.
+   */
+  virtual void
+  setData(InlineKey data_key, std::shared_ptr<Object> data, StateType state_type,
+          LifeSpan life_span = LifeSpan::FilterChain,
+          StreamSharingMayImpactPooling stream_sharing = StreamSharingMayImpactPooling::None) PURE;
+
   /**
    * @param data_name the name of the data being looked up (mutable/readonly).
    * @return a typed pointer to the stored data or nullptr if the data does not exist or the data
@@ -182,12 +209,27 @@ class FilterState {
     return dynamic_cast<const T*>(getDataReadOnlyGeneric(data_name));
   }
 
+  /**
+   * @param data_key the handle of the data being looked up (mutable/readonly).
+   * @return a typed pointer to the stored data or nullptr if the data does not exist or the data
+   * type does not match the expected type.
+   */
+  template <typename T> const T* getDataReadOnly(InlineKey data_key) const {
+    return dynamic_cast<const T*>(getDataReadOnlyGeneric(data_key));
+  }
+
   /**
    * @param data_name the name of the data being looked up (mutable/readonly).
    * @return a const pointer to the stored data or nullptr if the data does not exist.
    */
   virtual const Object* getDataReadOnlyGeneric(absl::string_view data_name) const PURE;
 
+  /**
+   * @param data_key the handle of the data being looked up (mutable/readonly).
+   * @return a const pointer to the stored data or nullptr if the data does not exist.
+   */
+  virtual const Object* getDataReadOnlyGeneric(InlineKey data_key) const PURE;
+
   /**
    * @param data_name the name of the data being looked up (mutable/readonly).
    * @return a typed pointer to the stored data or nullptr if the data does not exist or the data
@@ -197,18 +239,39 @@ class FilterState {
     return dynamic_cast<T*>(getDataMutableGeneric(data_name));
   }
 
+  /**
+   * @param data_name the name of the data being looked up (mutable/readonly).
+   * @return a typed pointer to the stored data or nullptr if the data does not exist or the data
+   * type does not match the expected type.
+   */
+  template <typename T> T* getDataMutable(InlineKey data_key) {
+    return dynamic_cast<T*>(getDataMutableGeneric(data_key));
+  }
+
   /**
    * @param data_name the name of the data being looked up (mutable/readonly).
    * @return a pointer to the stored data or nullptr if the data does not exist.
    */
   virtual Object* getDataMutableGeneric(absl::string_view data_name) PURE;
 
+  /**
+   * @param data_key the handle of the data being looked up (mutable/readonly).
+   * @return a pointer to the stored data or nullptr if the data does not exist.
+   */
+  virtual Object* getDataMutableGeneric(InlineKey data_key) PURE;
+
   /**
    * @param data_name the name of the data being looked up (mutable/readonly).
    * @return a shared pointer to the stored data or nullptr if the data does not exist.
    */
   virtual std::shared_ptr<Object> getDataSharedMutableGeneric(absl::string_view data_name) PURE;
 
+  /**
+   * @param data_key the handle of the data being looked up (mutable/readonly).
+   * @return a shared pointer to the stored data or nullptr if the data does not exist.
+   */
+  virtual std::shared_ptr<Object> getDataSharedMutableGeneric(InlineKey data_key) PURE;
+
   /**
    * @param data_name the name of the data being probed.
    * @return Whether data of the type and name specified exists in the
@@ -218,10 +281,34 @@ class FilterState {
     return getDataReadOnly<T>(data_name) != nullptr;
   }
 
+  /**
+   * @param data_key the handle of the data being probed.
+   * @return Whether data of the type and name specified exists in the
+   * data store.
+   */
+  template <typename T> bool hasData(InlineKey data_key) const {
+    return getDataReadOnly<T>(data_key) != nullptr;
+  }
+
+  /**
+   * @param data_name the name of the data being probed.
+   * @return Whether data of any type and the name specified exists in the
+   * data store.
+   */
+  virtual bool hasDataGeneric(absl::string_view data_name) const PURE;
+
+  /**
+   * @param data_key the handle of the data being probed.
+   * @return Whether data of any type and the name specified exists in the
+   * data store.
+   */
+  virtual bool hasDataGeneric(InlineKey data_key) const PURE;
+
   /**
    * @param data_name the name of the data being probed.
    * @return Whether data of any type and the name specified exists in the
    * data store.
+   * NOTE: Please use hasDataUnTyped() instead.
    */
   virtual bool hasDataWithName(absl::string_view data_name) const PURE;
 

source/common/common/BUILD

@@ -592,8 +592,22 @@ envoy_cc_library(
         "abseil_node_hash_set",
     ],
     deps = [
+        "//envoy/common:optref_lib",
         "//source/common/common:assert_lib",
         "//source/common/common:macros",
         "//source/common/common:utility_lib",
     ],
 )
+
+envoy_cc_library(
+    name = "inline_map_registry",
+    srcs = [
+        "inline_map_registry.cc",
+    ],
+    hdrs = [
+        "inline_map_registry.h",
+    ],
+    deps = [
+        ":inline_map",
+    ],
+)

source/common/common/inline_map.h

@@ -1,7 +1,9 @@
 #pragma once
 
+#include <atomic>
 #include <cstddef>
 #include <cstdint>
+#include <cstring>
 #include <list>
 #include <memory>
 
@@ -125,13 +127,9 @@ template <class StorageKey> class InlineMapDescriptor {
 
   /**
    * Finalize this descriptor. No further changes are allowed after this point. This guarantees that
-   * all map created by the process have the same variable size and custom inline key adding. This
-   * method should only be called once.
+   * all map created by the process have the same variable size and custom inline key adding.
    */
-  void finalize() {
-    ASSERT(!finalized_, "Cannot finalize() an already finalized descriptor");
-    finalized_ = true;
-  }
+  void finalize() { finalized_ = true; }
 
   /**
    * @return true if the descriptor is finalized.
@@ -178,7 +176,12 @@ template <class StorageKey> class InlineMapDescriptor {
     return inline_keys_map_.size();
   }
 
-  bool finalized_{};
+  // The finalize()/finalized() methods of cross-modules descriptor may be called at same time
+  // from different threads. So we need to use atomic to protect the finalized_ flag.
+  // This is only happens in the multiple threads tests because all cross-modules descriptors
+  // should be finalized when the first server instance is initialized in formal running.
+  std::atomic<bool> finalized_{};
+
   InlineKeys inline_keys_;
   InlineKeysMap inline_keys_map_;
 };
@@ -414,41 +417,42 @@ template <class Key, class Value> class InlineMap : public InlineStorage {
     return inline_entries_[handle.inlineId()];
   }
 
-  /**
-   * Create an inline map with the given descriptor. If the descriptor is not finalized, it will be
-   * finalized before creating the inline map.
-   * @param descriptor the descriptor that contains the inline keys.
-   * @return the created inline map.
-   */
-  static std::unique_ptr<InlineMap> create(TypedInlineMapDescriptor& descriptor) {
-    if (!descriptor.finalized()) {
-      // Call finalize() to make sure that the descriptor is finalized and no any new inline
-      // keys could be registered.
-      descriptor.finalize();
-    }
-
-    return std::unique_ptr<InlineMap>(new ((descriptor.inlineKeysNum() * sizeof(Value)))
-                                          InlineMap(descriptor));
+  InlineMap(const TypedInlineMapDescriptor& descriptor) : descriptor_(descriptor) {
+    ASSERT(descriptor_.finalized(), "Cannot create inline map before finalize()");
   }
 
 private:
+  using StoragePtr = std::unique_ptr<uint8_t[]>;
   friend class InlineMapDescriptor<Key>;
 
-  InlineMap(TypedInlineMapDescriptor& descriptor) : descriptor_(descriptor) {
-    // Initialize the inline entries to nullptr.
-    uint64_t inline_keys_num = descriptor_.inlineKeysNum();
-    inline_entries_valid_.resize(inline_keys_num, false);
-    memset(inline_entries_storage_, 0, inline_keys_num * sizeof(Value));
-    inline_entries_ = reinterpret_cast<Value*>(inline_entries_storage_);
+  void lazyAllocateMemoryInlineEntries() {
+    if (inline_entries_storage_ != nullptr) {
+      return;
+    }
+
+    const uint64_t inline_keys_num = descriptor_.inlineKeysNum();
+    const uint64_t value_memory_size = inline_keys_num * sizeof(Value);
+    const uint64_t flag_memory_size = inline_keys_num * sizeof(bool);
+    const uint64_t memory_size = value_memory_size + flag_memory_size;
+
+    inline_entries_storage_.reset(new uint8_t[memory_size]);
+    memset(inline_entries_storage_.get(), 0, memory_size);
+
+    inline_entries_ = reinterpret_cast<Value*>(inline_entries_storage_.get());
+    inline_entries_valid_ =
+        reinterpret_cast<bool*>(inline_entries_storage_.get() + value_memory_size);
   }
 
   template <class SetValue> void resetInlineMapEntry(uint64_t inline_entry_id, SetValue&& value) {
     ASSERT(inline_entry_id < descriptor_.inlineKeysNum());
+    // Only allocate the memory for inline entries when the first inline entry is added.
+    lazyAllocateMemoryInlineEntries();
 
     clearInlineMapEntry(inline_entry_id);
 
     // Construct the new entry in the inline array.
     new (inline_entries_ + inline_entry_id) Value(std::forward<SetValue>(value));
+
     setInlineEntryValid(inline_entry_id, true);
   }
 
@@ -484,10 +488,16 @@ template <class Key, class Value> class InlineMap : public InlineStorage {
 
   bool inlineEntryValid(uint64_t inline_entry_id) const {
     ASSERT(inline_entry_id < descriptor_.inlineKeysNum());
+
+    if (inline_entries_storage_ == nullptr) {
+      // If the inline entries storage is not allocated, then the inline entry is not valid.
+      return false;
+    }
     return inline_entries_valid_[inline_entry_id];
   }
   void setInlineEntryValid(uint64_t inline_entry_id, bool flag) {
     ASSERT(inline_entry_id < descriptor_.inlineKeysNum());
+    ASSERT(inline_entries_storage_ != nullptr);
 
     if (flag) {
       inline_entries_size_++;
@@ -506,20 +516,13 @@ template <class Key, class Value> class InlineMap : public InlineStorage {
   // This is the underlay hash map for the dynamic map entries.
   DynamicHashMap dynamic_entries_;
 
-  // These are flags to indicate if the inline entries are valid.
-  // TODO(wbpcode): this will add additional one time memory allocation when constructing inline
-  // map. It is possible to use memory in the inline_entries_storage_ to store the flags in the
-  // future.
-  std::vector<bool> inline_entries_valid_;
-
   uint64_t inline_entries_size_{};
 
   Value* inline_entries_{};
+  // These are flags to indicate if the inline entries are valid.
+  bool* inline_entries_valid_{};
 
-  // This should be the last member of the class and no member should be added after this.
-  uint8_t inline_entries_storage_[];
+  StoragePtr inline_entries_storage_;
 };
 
-template <class Key, class Value> using InlineMapPtr = std::unique_ptr<InlineMap<Key, Value>>;
-
 } // namespace Envoy

source/common/common/inline_map_registry.cc

@@ -0,0 +1,51 @@
+#include "source/common/common/inline_map_registry.h"
+
+namespace Envoy {
+
+InlineMapRegistry::Descriptor* InlineMapRegistry::createDescriptor(absl::string_view scope_name) {
+  // This should never be triggered because the developer should ensure all used/managed
+  // descriptors are registered before main() function by static variable initialization.
+  RELEASE_ASSERT(!finalized_, "Registry is finalized, cannot create new descriptor");
+
+  // Create the descriptor and insert it into the registry.
+  auto descriptor = new Descriptor();
+  auto [_, inserted] = descriptors_.emplace(scope_name, descriptor);
+
+  // Check whether the descriptor is created repeatedly.
+  RELEASE_ASSERT(inserted, fmt::format("Create descriptor {} repeatedly", scope_name));
+
+  return descriptor;
+}
+
+const InlineMapRegistry::Descriptors& InlineMapRegistry::descriptors() const {
+  RELEASE_ASSERT(finalized_, "Registry is not finalized, cannot get registry");
+  return descriptors_;
+}
+
+void InlineMapRegistry::finalize(const ScopeInlineKeysVector& scope_inline_keys_vector) {
+  // Call once to finalize the registry and all managed descriptors. This is used to ensure
+  // the registry is finalized only once on multi-thread environment.
+  std::call_once(once_flag_, [this, &scope_inline_keys_vector]() {
+    finalized_ = true;
+
+    // Add inline keys to the descriptor based on the scope name. This is last chance to add
+    // inline keys to the descriptor.
+    for (const auto& inline_keys : scope_inline_keys_vector) {
+      auto iter = descriptors_.find(inline_keys.name);
+      if (iter == descriptors_.end()) {
+        // Skip the scope that is not registered.
+        ENVOY_LOG_MISC(warn, "Skip the scope {} that is not registered", inline_keys.name);
+        continue;
+      }
+
+      for (const auto& key : inline_keys.keys) {
+        iter->second->addInlineKey(key);
+      }
+    }
+
+    // Finalize all managed descriptors.
+    std::for_each(descriptors_.begin(), descriptors_.end(), [](auto& p) { p.second->finalize(); });
+  });
+}
+
+} // namespace Envoy