[GitHub] [trafficserver] ywkaras commented on a change in pull request #6412: Optimize data structure for client SSL_CTX objects.

[GitBox <gi...@apache.org>]

ywkaras commented on a change in pull request #6412:
URL: https://github.com/apache/trafficserver/pull/6412#discussion_r566322072



##########
File path: iocore/net/P_SSLConfig.h
##########
@@ -42,6 +50,214 @@
 
 #include "P_SSLUtils.h"
 
+namespace SSLUtilsImpl
+{
+// An array of const instances of T.  T must be copyable.  The length of the array is fixed at the time of instance
+// creation.  The array associated with an instance will have the same value as the array passed to the make()
+// static member function.
+template <typename T, bool THasEq = false> class DurableConstArray
+{
+public:
+  // Lifetime of array associated with an instance of this class.
+  enum Duration {
+    PERMANENT,         // Longer than this instance or instance this instance is copied or moved to.
+    UNOWNED_TRANSIENT, // Only guaranteed to be as long as this instance.
+    OWNED_IN_HEAP      // Dynamically allocated and deleted when this instance is destroyed.
+  };
+
+  DurableConstArray() {}
+
+  ~DurableConstArray()
+  {
+    if (OWNED_IN_HEAP == _duration) {
+      for (std::size_t i = 0; i < _size; ++i) {
+        _ptr[i].~T();
+      }
+      delete[] reinterpret_cast<const char *>(_ptr);
+    }
+  }
+
+  // Use this member function for construction.
+  static DurableConstArray
+  make(Duration d, T const *ptr, std::size_t size)
+  {
+    DurableConstArray dc;
+    dc._ptr      = size ? ptr : nullptr;
+    dc._size     = ptr ? size : 0;
+    dc._duration = dc._size ? d : PERMANENT;
+    return dc;
+  }
+
+  DurableConstArray(const DurableConstArray &that)
+  {
+    _size = that._size;
+    if (PERMANENT == that._duration) {
+      _duration = PERMANENT;
+      _ptr      = that._ptr;
+    } else {
+      _duration     = OWNED_IN_HEAP;
+      char *p       = new char[sizeof(T) * _size];
+      _ptr          = reinterpret_cast<T *>(p);
+      std::size_t i = 0;
+      try {
+        for (; i < _size; ++i, p += sizeof(T)) {
+          ::new (p) T(that._ptr[i]);
+        }
+      } catch (...) {
+        while (i) {
+          _ptr[--i].~T();
+        }
+        throw;
+      }
+    }
+  }
+
+  DurableConstArray(DurableConstArray &&that)
+  {
+    _duration = that._duration;
+    _ptr      = that._ptr;
+    _size     = that._size;
+
+    ::new (&that) DurableConstArray();
+  }
+
+  DurableConstArray &
+  operator=(const DurableConstArray &that)
+  {
+    if (this != &that) {
+      this->~DurableConstArray();
+
+      ::new (this) DurableConstArray(that);
+    }
+    return *this;
+  }
+
+  DurableConstArray &
+  operator=(DurableConstArray &&that)
+  {
+    this->~DurableConstArray();
+
+    _duration = that._duration;
+    _ptr      = that._ptr;
+    _size     = that._size;
+
+    ::new (&that) DurableConstArray();
+
+    return *this;
+  }
+
+  Duration
+  duration() const
+  {
+    return _duration;
+  }
+  T const *
+  data() const
+  {
+    return _ptr;
+  }
+  std::size_t
+  size() const
+  {
+    return _size;
+  }
+
+  friend typename std::enable_if<THasEq, bool>::type
+  operator==(DurableConstArray const &a, DurableConstArray const &b)
+  {
+    if (a._size != b._size) {
+      return false;
+    }
+    if (a._ptr == b._ptr) {
+      return true;
+    }
+    if (!(a._ptr && b._ptr)) {
+      return false;
+    }
+    for (std::size_t i = 0; i < a._size; ++i) {
+      if (a._ptr[i] != b._ptr[i]) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  friend typename std::enable_if<THasEq, bool>::type
+  operator!=(DurableConstArray const &a, DurableConstArray const &b)
+  {
+    return !(a == b);
+  }
+
+private:
+  Duration _duration{PERMANENT};
+  T const *_ptr{nullptr}; // Array associated with this instance.
+  std::size_t _size{0};   // Dimenstion of array.
+};
+
+// A Key class for the unordered_map instances below that avoids unnecessary heap use.
+class TwoCStrKey
+{
+public:
+  TwoCStrKey() {}
+
+  TwoCStrKey(char const *major, char const *minor) : _major(_make(major)), _minor(_make(minor)) {}
+
+  friend bool
+  operator==(TwoCStrKey const &a, TwoCStrKey const &b)
+  {
+    if (a._major != b._major) {
+      return false;
+    }
+    return a._minor == b._minor;
+  }
+
+  std::string_view
+  major() const
+  {
+    return std::string_view(_major.data(), _major.size());
+  }
+  std::string_view
+  minor() const
+  {
+    return std::string_view(_minor.data(), _minor.size());
+  }
+
+  friend struct TwoCStrKeyHash;
+
+private:
+  using Comp = DurableConstArray<char, true>;
+
+  Comp _major, _minor;

Review comment:
       The main issue I'm trying to address is that the key for the unordered_map (generally) needs to use heap allocation for keys in the unordered_map, but not for keys passed to a find() on the unordered_map.  unordered_map has a fix for this in C++20, but it's not simple either:  https://en.cppreference.com/w/cpp/container/unordered_map/find .
   
   PERMANENT duration is not used in this case, so duration could be replaced with a simple owned_in_heap flag.  PERMANENT avoids unnecessary heap allocation of keys in the unordered_map as well.  I included PERMANENT because it could be useful if DurableConstArray at some point became a TS utility (in include/tscpp/util).  This is also why I made DurableConstArray a template rather than a simple class with T == char.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org