Automatic caching of parsed STRING config values (#12735)

This introduces ParsedConfigCache in HttpConfig to automatically cache
parsed results for STRING configs that require expensive parsing. When
TSHttpTxnConfigStringSet() is called for configs like
negative_caching_list, insert_forwarded, server_session_sharing.match,
negative_revalidating_list, or host_res_data (ip_resolve), the parsing
now happens once per unique value and is cached for subsequent calls.
This optimization is transparent to plugins - they call
TSHttpTxnConfigStringSet() as usual and automatically benefit from the
caching.

Fixes: #12292

Author: bneradt

include/proxy/http/HttpConfig.h

@@ -37,9 +37,12 @@
 #include <cstdio>
 #include <bitset>
 #include <map>
+#include <unordered_map>
 #include <cctype>
 #include <string_view>
 #include <chrono>
+#include <functional>
+#include <variant>
 
 #include "iocore/eventsystem/IOBuffer.h"
 #include "swoc/swoc_ip.h"
@@ -54,8 +57,10 @@
 #include "iocore/net/ConnectionTracker.h"
 #include "iocore/net/SessionSharingAPIEnums.h"
 #include "records/RecProcess.h"
+#include "tsutil/Bravo.h"
 #include "tsutil/ts_ip.h"
 #include "tsutil/Metrics.h"
+#include "ts/apidefs.h"
 
 using ts::Metrics;
 
@@ -852,6 +857,74 @@ struct HttpConfigParams : public ConfigInfo {
   HttpConfigParams &operator=(const HttpConfigParams &) = delete;
 };
 
+/////////////////////////////////////////////////////////////
+//
+// class ParsedConfigCache
+//
+/////////////////////////////////////////////////////////////
+
+/** Cache for pre-parsed string config values.
+ *
+ * Some overridable STRING configs require parsing (e.g., status code lists,
+ * host resolution preferences). Parsing can be non-trivial. This cache stores
+ * parsed results so repeated calls to TSHttpTxnConfigStringSet() with the same
+ * value don't re-parse.
+ *
+ * The static lookup() method handles everything: check cache, parse if needed,
+ * store in cache, and return the result.
+ */
+class ParsedConfigCache
+{
+public:
+  /** Pre-parsed representations for configs that need special parsing.
+   *
+   * Uses std::variant since each cache entry only stores one type of parsed
+   * result. The conf_value_storage string is always present as it owns the
+   * string data that the parsed structures may reference.
+   */
+  struct ParsedValue {
+    std::string conf_value_storage{}; // Owns the string data.
+    std::variant<std::monostate, HostResData, HttpStatusCodeList, HttpForwarded::OptionBitSet, MgmtByte> parsed{};
+  };
+
+  /** Return the parsed value for the configuration.
+   *
+   * On first call for a given (key, value) pair, parses the value and caches it.
+   * Subsequent calls return the cached result.
+   *
+   * @param key The config key being referenced.
+   * @param value The string value to parse.
+   * @return Reference to the cached parsed value.
+   */
+  static const ParsedValue &lookup(TSOverridableConfigKey key, std::string_view value);
+
+private:
+  ParsedConfigCache() = default;
+
+  // Enforce singleton pattern.
+  ParsedConfigCache(const ParsedConfigCache &)            = delete;
+  ParsedConfigCache &operator=(const ParsedConfigCache &) = delete;
+  ParsedConfigCache(ParsedConfigCache &&)                 = delete;
+  ParsedConfigCache &operator=(ParsedConfigCache &&)      = delete;
+
+  static ParsedConfigCache &instance();
+
+  const ParsedValue &lookup_impl(TSOverridableConfigKey key, std::string_view value);
+  ParsedValue        parse(TSOverridableConfigKey key, std::string_view value);
+
+  // Custom hash for the cache key.
+  struct CacheKeyHash {
+    std::size_t
+    operator()(const std::pair<TSOverridableConfigKey, std::string> &k) const
+    {
+      return std::hash<int>()(static_cast<int>(k.first)) ^ (std::hash<std::string>()(k.second) << 1);
+    }
+  };
+
+  std::unordered_map<std::pair<TSOverridableConfigKey, std::string>, ParsedValue, CacheKeyHash> _cache;
+  ts::bravo::shared_mutex                                                                       _mutex;
+};
+
 /////////////////////////////////////////////////////////////
 //
 // class HttpConfig

include/proxy/http/OverridableConfigDefs.h

@@ -90,6 +90,20 @@
 
   See the X-Macro Dispatch doxygen documentation in InkAPI.cc for more details.
 
+  @section caching Automatic Caching for STRING Configs
+
+  Some STRING configs require parsing that is more expensive than a simple
+  string copy (e.g., parsing status code lists or host resolution preferences).
+  For these configs, TSHttpTxnConfigStringSet() automatically uses
+  ParsedConfigCache (defined in HttpConfig.h) to cache parsed results.
+
+  This means the parsing only happens once per unique (config_key, value) pair.
+  Subsequent calls with the same value use the cached result directly.
+  HTTP_NEGATIVE_CACHING_LIST is an example configuration that takes advantage
+  of this. This optimization is transparent to API users - they just call
+  TSHttpTxnConfigStringSet() as usual and get the performance benefit
+  automatically.
+
   @section none_configs Note on CONV=NONE
 
   Some SSL string configs use NONE because they bypass _conf_to_memberp()

src/api/InkAPI.cc

@@ -7309,6 +7309,19 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr
   conv      = nullptr;
 
   switch (conf) {
+    // This uses OVERRIDABLE_CONFIGS to generate cases for each config.
+    // For example:
+    //
+    // case TS_CONFIG_HTTP_CHUNKING_ENABLED:
+    //   ret = _memberp_to_generic(&overridableHttpConfig->chunking_enabled, conv);
+    //   break;
+    // case TS_CONFIG_HTTP_DOWN_SERVER_CACHE_TIME:
+    //   conv = &HttpDownServerCacheTimeConv;
+    //   ret = &overridableHttpConfig->down_server_timeout;
+    //   break;
+    //
+    // ... ~130 more cases, one per overridable config ...
+    //
     OVERRIDABLE_CONFIGS(_CONF_CASE_DISPATCH)
   case TS_CONFIG_NULL:
   case TS_CONFIG_LAST_ENTRY:
@@ -7472,19 +7485,15 @@ TSHttpTxnConfigStringSet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
     break;
   case TS_CONFIG_HTTP_INSERT_FORWARDED:
     if (value && length > 0) {
-      swoc::LocalBufferWriter<1024> error;
-      HttpForwarded::OptionBitSet   bs = HttpForwarded::optStrToBitset(std::string_view(value, length), error);
-      if (!error.size()) {
-        s->t_state.my_txn_conf().insert_forwarded = bs;
-      } else {
-        Error("HTTP %.*s", static_cast<int>(error.size()), error.data());
-      }
+      auto &parsed                              = ParsedConfigCache::lookup(conf, std::string_view(value, length));
+      s->t_state.my_txn_conf().insert_forwarded = std::get<HttpForwarded::OptionBitSet>(parsed.parsed);
     }
     break;
   case TS_CONFIG_HTTP_SERVER_SESSION_SHARING_MATCH:
     if (value && length > 0) {
-      HttpConfig::load_server_session_sharing_match(value, s->t_state.my_txn_conf().server_session_sharing_match);
-      s->t_state.my_txn_conf().server_session_sharing_match_str = const_cast<char *>(value);
+      auto &parsed                                              = ParsedConfigCache::lookup(conf, std::string_view(value, length));
+      s->t_state.my_txn_conf().server_session_sharing_match     = std::get<MgmtByte>(parsed.parsed);
+      s->t_state.my_txn_conf().server_session_sharing_match_str = const_cast<char *>(parsed.conf_value_storage.data());
     }
     break;
   case TS_CONFIG_SSL_CLIENT_VERIFY_SERVER_POLICY:
@@ -7527,23 +7536,22 @@ TSHttpTxnConfigStringSet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
     break;
   case TS_CONFIG_HTTP_NEGATIVE_CACHING_LIST:
     if (value && length > 0) {
-      OverridableHttpConfigParams *target      = &s->t_state.my_txn_conf();
-      target->negative_caching_list.conf_value = const_cast<char *>(value);
-      return _eval_conv(target, conf, value, length);
+      auto &parsed                                   = ParsedConfigCache::lookup(conf, std::string_view(value, length));
+      s->t_state.my_txn_conf().negative_caching_list = std::get<HttpStatusCodeList>(parsed.parsed);
     }
     break;
   case TS_CONFIG_HTTP_NEGATIVE_REVALIDATING_LIST:
     if (value && length > 0) {
-      OverridableHttpConfigParams *target           = &s->t_state.my_txn_conf();
-      target->negative_revalidating_list.conf_value = const_cast<char *>(value);
-      return _eval_conv(target, conf, value, length);
+      auto &parsed                                        = ParsedConfigCache::lookup(conf, std::string_view(value, length));
+      s->t_state.my_txn_conf().negative_revalidating_list = std::get<HttpStatusCodeList>(parsed.parsed);
     }
     break;
   case TS_CONFIG_HTTP_HOST_RESOLUTION_PREFERENCE:
     if (value && length > 0) {
-      s->t_state.my_txn_conf().host_res_data.conf_value = const_cast<char *>(value);
+      auto &parsed                           = ParsedConfigCache::lookup(conf, std::string_view(value, length));
+      s->t_state.my_txn_conf().host_res_data = std::get<HostResData>(parsed.parsed);
     }
-    [[fallthrough]];
+    break;
   default: {
     if (value && length > 0) {
       return _eval_conv(&(s->t_state.my_txn_conf()), conf, value, length);

src/api/InkAPITest.cc

@@ -8781,9 +8781,11 @@ REGRESSION_TEST(SDK_API_OVERRIDABLE_CONFIGS)(RegressionTest *test, int /* atype
     case TS_RECORDDATATYPE_STRING:
       TSHttpTxnConfigStringSet(txnp, key, test_string, -1);
       TSHttpTxnConfigStringGet(txnp, key, &sval_read, &len);
-      if (test_string != sval_read) {
-        SDK_RPRINT(test, "TSHttpTxnConfigStringSet", "TestCase1", TC_FAIL, "Failed on %s, %s != %s", conf.data(), sval_read,
-                   test_string);
+      // Compare string content, not pointers - the implementation may store
+      // a copy of the string (e.g., in ParsedConfigCache for efficiency).
+      if (sval_read == nullptr || std::string_view(test_string) != std::string_view(sval_read, len)) {
+        SDK_RPRINT(test, "TSHttpTxnConfigStringSet", "TestCase1", TC_FAIL, "Failed on %s, %s != %s", conf.data(),
+                   sval_read ? sval_read : "(null)", test_string);
         success = false;
         continue;
       }

src/proxy/http/HttpConfig.cc

@@ -694,6 +694,103 @@ const MgmtConverter HttpStatusCodeList::Conv{
   }};
 // clang-format on
 
+/////////////////////////////////////////////////////////////
+//
+// ParsedConfigCache implementation
+//
+/////////////////////////////////////////////////////////////
+
+ParsedConfigCache &
+ParsedConfigCache::instance()
+{
+  static ParsedConfigCache inst;
+  return inst;
+}
+
+const ParsedConfigCache::ParsedValue &
+ParsedConfigCache::lookup(TSOverridableConfigKey key, std::string_view value)
+{
+  return instance().lookup_impl(key, value);
+}
+
+const ParsedConfigCache::ParsedValue &
+ParsedConfigCache::lookup_impl(TSOverridableConfigKey key, std::string_view value)
+{
+  auto cache_key = std::make_pair(key, std::string(value));
+
+  // Fast path: check cache under read lock.
+  {
+    ts::bravo::shared_lock lock(_mutex);
+    auto                   it = _cache.find(cache_key);
+    if (it != _cache.end()) {
+      return it->second;
+    }
+  }
+
+  // Slow path: parse and insert under write lock.
+  std::unique_lock lock(_mutex);
+
+  // Double-check after acquiring write lock.
+  auto it = _cache.find(cache_key);
+  if (it != _cache.end()) {
+    return it->second;
+  }
+
+  // Parse and insert.
+  auto [inserted_it, success] = _cache.emplace(cache_key, parse(key, value));
+  return inserted_it->second;
+}
+
+ParsedConfigCache::ParsedValue
+ParsedConfigCache::parse(TSOverridableConfigKey key, std::string_view value)
+{
+  ParsedValue result;
+
+  // Store the string value - the parsed structures may reference this.
+  result.conf_value_storage = std::string(value);
+
+  switch (key) {
+  case TS_CONFIG_HTTP_HOST_RESOLUTION_PREFERENCE: {
+    HostResData host_res_data{};
+    parse_host_res_preference(result.conf_value_storage.c_str(), host_res_data.order);
+    host_res_data.conf_value = result.conf_value_storage.data();
+    result.parsed            = host_res_data;
+    break;
+  }
+
+  case TS_CONFIG_HTTP_NEGATIVE_CACHING_LIST:
+  case TS_CONFIG_HTTP_NEGATIVE_REVALIDATING_LIST: {
+    HttpStatusCodeList status_code_list{};
+    status_code_list.conf_value = result.conf_value_storage.data();
+    HttpStatusCodeList::Conv.store_string(&status_code_list, result.conf_value_storage);
+    result.parsed = status_code_list;
+    break;
+  }
+
+  case TS_CONFIG_HTTP_INSERT_FORWARDED: {
+    swoc::LocalBufferWriter<1024> error;
+    result.parsed = HttpForwarded::optStrToBitset(result.conf_value_storage, error);
+    if (error.size()) {
+      Error("HTTP %.*s", static_cast<int>(error.size()), error.data());
+    }
+    break;
+  }
+
+  case TS_CONFIG_HTTP_SERVER_SESSION_SHARING_MATCH: {
+    MgmtByte server_session_sharing_match{0};
+    HttpConfig::load_server_session_sharing_match(result.conf_value_storage, server_session_sharing_match);
+    result.parsed = server_session_sharing_match;
+    break;
+  }
+
+  default:
+    // No special parsing needed for this config.
+    break;
+  }
+
+  return result;
+}
+
 /** Template for creating conversions and initialization for @c std::chrono based configuration variables.
  *
  * @tparam V The exact type of the configuration variable.

src/shared/overridable_txn_vars.cc

@@ -66,14 +66,24 @@ static_assert(sizeof(xmacro_enum_order) / sizeof(xmacro_enum_order[0]) == TS_CON
 static_assert(check_xmacro_order(xmacro_enum_order),
               "OVERRIDABLE_CONFIGS order must match TSOverridableConfigKey enum order in apidefs.h.in. "
               "Ensure entries are in the same order in both files.");
+// ============================================================================
+// End of compile-time validation.
+// ============================================================================
 
 // ============================================================================
-// String-to-enum mapping generated from X-macro.
+// Configuration string name to enum and type mapping.
 // ============================================================================
 
 // clang-format off
 const std::unordered_map<std::string_view, std::tuple<const TSOverridableConfigKey, const TSRecordDataType>>
   ts::Overridable_Txn_Vars({
+
+/** Use OVERRIDABLE_CONFIGS to populate the map with entries like:
+ * ...
+ * "proxy.config.http.chunking_enabled", {TS_CONFIG_HTTP_CHUNKING_ENABLED, TS_RECORDDATATYPE_INT}
+ * "proxy.config.http.negative_caching_list", {TS_CONFIG_HTTP_NEGATIVE_CACHING_LIST, TS_RECORDDATATYPE_STRING}
+ * ...
+ */
 #define X_TXN_VAR(CONFIG_KEY, MEMBER, RECORD_NAME, DATA_TYPE, CONV) \
     {RECORD_NAME, {TS_CONFIG_##CONFIG_KEY, TS_RECORDDATATYPE_##DATA_TYPE}},
     OVERRIDABLE_CONFIGS(X_TXN_VAR)