added additional instance cache metric (#924)

added additional instance cache 'kphp_instance_cache_data_size' metric

.github/workflows/Build.yml

@@ -140,9 +140,9 @@ jobs:
       run: |
         brew tap shivammathur/php
         brew update
-        brew install re2c cmake coreutils openssl libiconv re2 pcre yaml-cpp zstd googletest shivammathur/php/[email protected]
+        brew install re2c cmake coreutils openssl libiconv re2 pcre yaml-cpp zstd googletest shivammathur/php/[email protected] [email protected]
         brew link --overwrite --force shivammathur/php/[email protected]
-        /usr/local/Frameworks/Python.framework/Versions/3.11/bin/pip3 install jsonschema
+        /usr/local/Frameworks/Python.framework/Versions/3.12/bin/python3.12 -m pip install --upgrade pip --break-system-packages && /usr/local/Frameworks/Python.framework/Versions/3.12/bin/pip3 install jsonschema install --break-system-packages jsonschema
 
     - name: Run cmake
       run: cmake -DCMAKE_CXX_COMPILER=${{matrix.compiler}} -DCMAKE_CXX_STANDARD=${{matrix.cpp}} -DDOWNLOAD_MISSING_LIBRARIES=On -S $GITHUB_WORKSPACE -B ${{runner.workspace}}/build

.github/workflows/macos.yml

@@ -22,21 +22,20 @@ jobs:
 
     steps:
     - uses: actions/checkout@v3
-
-    - name: Get polyfills repo
-      uses: actions/checkout@v3
-      with:
-        repository: 'uber/h3'
-        path: 'h3'
-        ref: stable-3.x
+
+      # because of https://github.com/orgs/Homebrew/discussions/4612
+    - name: Check Environment
+      run: |
+        export HOMEBREW_NO_INSTALL_FROM_API=0
+        brew untap --force homebrew/cask
 
     - name: Setup Environment
       run: |
         brew tap shivammathur/php
         brew update
-        brew install re2c cmake coreutils openssl libiconv re2 pcre yaml-cpp zstd googletest shivammathur/php/[email protected]
+        brew install re2c cmake coreutils openssl libiconv re2 pcre yaml-cpp zstd googletest shivammathur/php/[email protected] [email protected]
         brew link --overwrite --force shivammathur/php/[email protected]
-        /usr/local/Frameworks/Python.framework/Versions/3.11/bin/pip3 install jsonschema
+        /usr/local/Frameworks/Python.framework/Versions/3.12/bin/python3.12 -m pip install --upgrade pip --break-system-packages && /usr/local/Frameworks/Python.framework/Versions/3.12/bin/pip3 install jsonschema install --break-system-packages jsonschema
       
     - name: Run cmake
       run: cmake -DCMAKE_CXX_COMPILER=${{matrix.compiler}} -DCMAKE_CXX_STANDARD=${{matrix.cpp}} -DDOWNLOAD_MISSING_LIBRARIES=On -S $GITHUB_WORKSPACE -B ${{runner.workspace}}/build

builtin-functions/kphp_toggles.txt

@@ -13,3 +13,5 @@ function set_json_log_demangle_stacktrace(bool $enable) ::: void;
 function set_use_updated_gmmktime(bool $enable) ::: void;
 
 function kphp_turn_on_host_tag_in_inner_statshouse_metrics_toggle();
+
+function kphp_extended_instance_cache_metrics_init(callable(string $key):string $normalization_function) ::: void;

compiler/code-gen/declarations.cpp

@@ -928,15 +928,19 @@ void ClassDeclaration::compile_accept_visitor_methods(CodeGenerator &W, ClassPtr
     compile_accept_visitor(W, klass, "ToArrayVisitor");
   }
 
-  if (klass->need_instance_memory_estimate_visitor) {
+  if (klass->need_instance_memory_estimate_visitor ||
+      // for kphp_instance_cache_value_size statshouse metrics
+      klass->need_instance_cache_visitors) {
     W << NL;
     compile_accept_visitor(W, klass, "InstanceMemoryEstimateVisitor");
   }
 
   if (klass->need_instance_cache_visitors) {
     W << NL;
     compile_accept_visitor(W, klass, "InstanceReferencesCountingVisitor");
+    W << NL;
     compile_accept_visitor(W, klass, "InstanceDeepCopyVisitor");
+    W << NL;
     compile_accept_visitor(W, klass, "InstanceDeepDestroyVisitor");
   }
 
@@ -1084,15 +1088,19 @@ void ClassMembersDefinition::compile(CodeGenerator &W) const {
     compile_generic_accept_instantiations(W, klass, "ToArrayVisitor");
   }
 
-  if (klass->need_instance_memory_estimate_visitor) {
+  if (klass->need_instance_memory_estimate_visitor ||
+      // for kphp_instance_cache_value_size statshouse metrics
+      klass->need_instance_cache_visitors) {
     W << NL;
     compile_generic_accept_instantiations(W, klass, "InstanceMemoryEstimateVisitor");
   }
 
   if (klass->need_instance_cache_visitors) {
     W << NL;
     compile_generic_accept_instantiations(W, klass, "InstanceReferencesCountingVisitor");
+    W << NL;
     compile_generic_accept_instantiations(W, klass, "InstanceDeepCopyVisitor");
+    W << NL;
     compile_generic_accept_instantiations(W, klass, "InstanceDeepDestroyVisitor");
   }
 

runtime/instance-cache.cpp

@@ -289,19 +289,19 @@ class InstanceCache {
     context_ = nullptr;
   }
 
-  bool store(const string &key, const InstanceCopyistBase &instance_wrapper, int64_t ttl) noexcept {
+  InstanceCacheOpStatus store(const string &key, const InstanceCopyistBase &instance_wrapper, int64_t ttl) noexcept {
     ic_debug("store '%s'\n", key.c_str());
     php_assert(current_ && context_);
     if (context_->memory_swap_required) {
-      return false;
+      return InstanceCacheOpStatus::memory_swap_required;
     }
 
     sync_delayed();
     // various service things that we can do without synchronization
     auto &data = current_->get_data(key);
     update_now();
     if (is_element_insertion_can_be_skipped(data, key)) {
-      return false;
+      return InstanceCacheOpStatus::skipped;
     }
 
     InstanceDeepCopyVisitor detach_processor{context_->memory_resource, ExtraRefCnt::for_instance_cache};
@@ -311,22 +311,26 @@ class InstanceCache {
     if (!inserted_element) {
       // failed to insert the element due to some problems (e.g. memory, depth limit)
       if (unlikely(!detach_processor.is_ok())) {
-        fire_warning(detach_processor, instance_wrapper.get_class());
-        return false;
+        if (detach_processor.is_memory_limit_exceeded()) {
+          fire_warning(instance_wrapper.get_class());
+          return InstanceCacheOpStatus::memory_limit_exceeded;
+        }
+
+        return InstanceCacheOpStatus::failed;
       }
       // failed to acquire a lock, save the instance into the script memory container, we'll try again later
       class_instance<DelayedInstance> delayed_instance;
       delayed_instance.alloc().get()->ttl = ttl;
       delayed_instance.get()->instance_wrapper = instance_wrapper.shallow_copy();
       storing_delayed_.set_value(key, std::move(delayed_instance));
       context_->stats.elements_storing_delayed_due_mutex.fetch_add(1, std::memory_order_relaxed);
-      return false;
+      return InstanceCacheOpStatus::delayed;
     }
     ic_debug("element '%s' was successfully inserted\n", key.c_str());
     context_->stats.elements_stored.fetch_add(1, std::memory_order_relaxed);
     // request_cache_ uses a script memory
     request_cache_.set_value(key, inserted_element);
-    return true;
+    return InstanceCacheOpStatus::success;
   }
 
   const InstanceCopyistBase *fetch(const string &key, bool even_if_expired) {
@@ -566,8 +570,8 @@ class InstanceCache {
           // failed to acquire an allocator lock; try later
           return;
         }
-        fire_warning(detach_processor, delayed_instance.instance_wrapper->get_class());
         if (detach_processor.is_memory_limit_exceeded()) {
+          fire_warning(delayed_instance.instance_wrapper->get_class());
           return;
         }
       } else {
@@ -634,11 +638,9 @@ class InstanceCache {
     return nullptr;
   }
 
-  void fire_warning(const InstanceDeepCopyVisitor &detach_processor, const char *class_name) noexcept {
-    if (detach_processor.is_memory_limit_exceeded()) {
-      php_warning("Memory limit exceeded on saving instance of class '%s' into cache", class_name);
-      context_->memory_swap_required = true;
-    }
+  void fire_warning(const char *class_name) noexcept {
+    php_warning("Memory limit exceeded on saving instance of class '%s' into cache", class_name);
+    context_->memory_swap_required = true;
   }
 
   SharedMemoryData *current_{nullptr};
@@ -676,7 +678,19 @@ class InstanceCache {
   size_t purge_shard_offset_{0};
 };
 
-bool instance_cache_store(const string &key, const InstanceCopyistBase &instance_wrapper, int64_t ttl) {
+std::string_view instance_cache_store_status_to_str(InstanceCacheOpStatus status) {
+  switch (status) {
+    case InstanceCacheOpStatus::success:                 return "success";
+    case InstanceCacheOpStatus::skipped:                 return "skipped";
+    case InstanceCacheOpStatus::memory_limit_exceeded:   return "memory_limit_exceeded";
+    case InstanceCacheOpStatus::delayed:                 return "delayed";
+    case InstanceCacheOpStatus::not_found:               return "not_found";
+    case InstanceCacheOpStatus::failed:                  return "failed";
+    default:                                             return "unknown";
+  }
+}
+
+InstanceCacheOpStatus instance_cache_store(const string &key, const InstanceCopyistBase &instance_wrapper, int64_t ttl) {
   return InstanceCache::get().store(key, instance_wrapper, ttl);
 }
 

runtime/instance-cache.h

@@ -20,11 +20,16 @@
 
 #include "runtime/instance-copy-processor.h"
 #include "runtime/kphp_core.h"
+#include "runtime/memory_usage.h"
 #include "runtime/shape.h"
+#include "server/statshouse/statshouse-manager.h"
+
+enum class InstanceCacheOpStatus;
 
 namespace impl_ {
 
-bool instance_cache_store(const string &key, const InstanceCopyistBase &instance_wrapper, int64_t ttl);
+std::string_view instance_cache_store_status_to_str(InstanceCacheOpStatus status);
+InstanceCacheOpStatus instance_cache_store(const string &key, const InstanceCopyistBase &instance_wrapper, int64_t ttl);
 const InstanceCopyistBase *instance_cache_fetch_wrapper(const string &key, bool even_if_expired);
 
 } // namespace impl_
@@ -59,6 +64,17 @@ enum class InstanceCacheSwapStatus {
   swap_is_finished, // swap succeeded
   swap_is_forbidden // swap is not possible - the memory is still being used
 };
+
+enum class InstanceCacheOpStatus {
+  success,
+  skipped,
+  memory_limit_exceeded,
+  memory_swap_required,
+  delayed,
+  not_found,
+  failed
+};
+
 // these function should be called from master
 InstanceCacheSwapStatus instance_cache_try_swap_memory();
 // these function should be called from master
@@ -70,14 +86,24 @@ void instance_cache_purge_expired_elements();
 
 void instance_cache_release_all_resources_acquired_by_this_proc();
 
+template<typename ClassInstanceType>
+void send_extended_instance_cache_stats_if_enabled(std::string_view op, InstanceCacheOpStatus status, const string &key, const ClassInstanceType &instance) {
+  if (StatsHouseManager::get().is_extended_instance_cache_stats_enabled()) {
+    int64_t size = f$estimate_memory_usage(key) + f$estimate_memory_usage(instance);
+    StatsHouseManager::get().add_extended_instance_cache_stats(op, impl_::instance_cache_store_status_to_str(status), key, size);
+  }
+}
+
 template<typename ClassInstanceType>
 bool f$instance_cache_store(const string &key, const ClassInstanceType &instance, int64_t ttl = 0) {
   static_assert(is_class_instance<ClassInstanceType>::value, "class_instance<> type expected");
   if (instance.is_null()) {
     return false;
   }
   InstanceCopyistImpl<ClassInstanceType> instance_wrapper{instance};
-  return impl_::instance_cache_store(key, instance_wrapper, ttl);
+  InstanceCacheOpStatus status = impl_::instance_cache_store(key, instance_wrapper, ttl);
+  send_extended_instance_cache_stats_if_enabled("store", status, key, instance);
+  return status == InstanceCacheOpStatus::success;
 }
 
 template<typename ClassInstanceType>
@@ -89,11 +115,15 @@ ClassInstanceType f$instance_cache_fetch(const string &class_name, const string
     if (auto wrapper = dynamic_cast<const InstanceCopyistImpl<ClassInstanceType> *>(base_wrapper)) {
       auto result = wrapper->get_instance();
       php_assert(!result.is_null());
+      send_extended_instance_cache_stats_if_enabled("fetch", InstanceCacheOpStatus::success, key, result);
       return result;
     } else {
+      send_extended_instance_cache_stats_if_enabled("fetch", InstanceCacheOpStatus::failed, key, ClassInstanceType{});
       php_warning("Trying to fetch incompatible instance class: expect '%s', got '%s'",
                   class_name.c_str(), base_wrapper->get_class());
     }
+  } else {
+    send_extended_instance_cache_stats_if_enabled("fetch", InstanceCacheOpStatus::not_found, key, ClassInstanceType{});
   }
   return {};
 }

runtime/interface.h

@@ -243,6 +243,12 @@ inline void f$kphp_turn_on_host_tag_in_inner_statshouse_metrics_toggle() {
   StatsHouseManager::get().turn_on_host_tag_toggle();
 }
 
+template <typename F>
+inline void f$kphp_extended_instance_cache_metrics_init(F &&callback) {
+  dl::CriticalSectionGuard guard;
+  StatsHouseManager::get().set_normalization_function(normalization_function{std::forward<F>(callback)});
+}
+
 int64_t f$numa_get_bound_node();
 
 bool f$extension_loaded(const string &extension);

server/statshouse/statshouse-manager.cpp

@@ -11,7 +11,6 @@
 #include "runtime/instance-cache.h"
 #include "server/job-workers/shared-memory-manager.h"
 #include "server/json-logger.h"
-#include "server/php-engine-vars.h"
 #include "server/php-runner.h"
 #include "server/server-config.h"
 #include "server/server-stats.h"
@@ -249,6 +248,12 @@ void StatsHouseManager::add_init_master_stats(uint64_t total_init_ns, uint64_t c
   client.metric("kphp_by_host_master_confdata_init_time", true).write_value(confdata_init_ns);
 }
 
+void StatsHouseManager::add_extended_instance_cache_stats(std::string_view type, std::string_view status, const string &key, uint64_t size) {
+  dl::CriticalSectionGuard guard;
+  string normalize_key = instance_cache_key_normalization_function(key);
+  client.metric("kphp_instance_cache_data_size", true).tag(type).tag(status).tag(normalize_key.c_str()).write_value(size);
+}
+
 void StatsHouseManager::add_job_workers_shared_memory_stats(const job_workers::JobStats &job_stats) {
   using namespace job_workers;
 

server/statshouse/statshouse-manager.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <cassert>
+#include <functional>
 
 #include "common/dl-utils-lite.h"
 #include "common/mixin/not_copyable.h"
@@ -14,6 +15,8 @@
 #include "server/workers-control.h"
 #include "server/workers-stats.h"
 
+using normalization_function = std::function<string(const string &)>;
+
 enum class script_error_t : uint8_t;
 
 class StatsHouseManager : vk::not_copyable {
@@ -42,6 +45,14 @@ class StatsHouseManager : vk::not_copyable {
     need_write_enable_tag_host = true;
   }
 
+  void set_normalization_function(normalization_function &&_function) {
+    this->instance_cache_key_normalization_function = std::move(_function);
+  }
+
+  bool is_extended_instance_cache_stats_enabled() {
+    return this->instance_cache_key_normalization_function != nullptr;
+  }
+
   void add_request_stats(uint64_t script_time_ns, uint64_t net_time_ns, script_error_t error, const memory_resource::MemoryStats &script_memory_stats,
                          uint64_t script_queries, uint64_t long_script_queries,
                          uint64_t script_user_time_ns, uint64_t script_system_time_ns,
@@ -66,9 +77,15 @@ class StatsHouseManager : vk::not_copyable {
    */
   void add_init_master_stats(uint64_t total_init_ns, uint64_t confdata_init_ns);
 
+  /**
+   * before calling the method, be sure to is_extended_instance_cache_stats_enabled() is true
+   */
+  void add_extended_instance_cache_stats(std::string_view type, std::string_view status, const string &key, uint64_t size = 0);
+
 private:
   StatsHouseClient client;
   bool need_write_enable_tag_host = false;
+  normalization_function instance_cache_key_normalization_function = nullptr;
 
   StatsHouseManager() = default;
   explicit StatsHouseManager(const std::string &ip, int port);