From 4fd655ffd1980f6d3b40023036c2feff8d6d51c6 Mon Sep 17 00:00:00 2001 From: Stanislav Eismont Date: Thu, 14 Sep 2023 12:22:10 +0300 Subject: [PATCH 1/2] fix function estimate_memory_usage(). Previously it shows incorrect results for both vector and map storages --- runtime/array.inl | 17 ++++------- runtime/array_decl.inl | 10 +++---- tests/phpt/memory_usage/10_arrays.php | 43 +++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 tests/phpt/memory_usage/10_arrays.php diff --git a/runtime/array.inl b/runtime/array.inl index b0830a8dd4..15c92ed4f0 100644 --- a/runtime/array.inl +++ b/runtime/array.inl @@ -129,7 +129,7 @@ uint32_t array::array_inner::choose_bucket(const array_inner_fields_for_map & } template -bool array::array_inner::is_vector() const { +bool array::array_inner::is_vector() const noexcept { return is_vector_internal; } @@ -203,12 +203,12 @@ const typename array::array_inner_fields_for_map &array::array_inner::fiel } template -size_t array::array_inner::sizeof_vector(uint32_t int_size) { +size_t array::array_inner::sizeof_vector(uint32_t int_size) noexcept { return sizeof(array_inner) + int_size * sizeof(T); } template -size_t array::array_inner::sizeof_map(uint32_t int_size) { +size_t array::array_inner::sizeof_map(uint32_t int_size) noexcept { return sizeof(array_inner_fields_for_map) + sizeof(array_inner) + int_size * sizeof(array_bucket); } @@ -611,13 +611,8 @@ bool array::array_inner::has_no_string_keys() const noexcept { } template -size_t array::array_inner::estimate_memory_usage() const { - int64_t int_elements = size; - const bool vector_structure = is_vector(); - if (vector_structure) { - return estimate_size(int_elements, vector_structure); - } - return estimate_size(++int_elements, vector_structure); +size_t array::array_inner::estimate_memory_usage() const noexcept { + return is_vector() ? sizeof_vector(buf_size) : sizeof_map(buf_size); } template @@ -781,7 +776,7 @@ void array::reserve(int64_t int_size, bool make_vector_if_possible) { } template -size_t array::estimate_memory_usage() const { +size_t array::estimate_memory_usage() const noexcept { return p->estimate_memory_usage(); } diff --git a/runtime/array_decl.inl b/runtime/array_decl.inl index 16ad2c61a4..01f1ba5bff 100644 --- a/runtime/array_decl.inl +++ b/runtime/array_decl.inl @@ -102,7 +102,7 @@ private: array_bucket entries[KPHP_ARRAY_TAIL_SIZE]; - inline bool is_vector() const __attribute__ ((always_inline)); + inline bool is_vector() const noexcept __attribute__ ((always_inline)); inline list_hash_entry *get_entry(entry_pointer_type pointer) const __attribute__ ((always_inline)); inline entry_pointer_type get_pointer(list_hash_entry *entry) const __attribute__ ((always_inline)); @@ -125,8 +125,8 @@ private: inline uint32_t choose_bucket(int64_t key) const noexcept __attribute__ ((always_inline)); inline uint32_t choose_bucket(const array_inner_fields_for_map &fields, int64_t key) const noexcept __attribute__ ((always_inline)); - inline static size_t sizeof_vector(uint32_t int_size) __attribute__((always_inline)); - inline static size_t sizeof_map(uint32_t int_size) __attribute__((always_inline)); + inline static size_t sizeof_vector(uint32_t int_size) noexcept __attribute__((always_inline)); + inline static size_t sizeof_map(uint32_t int_size) noexcept __attribute__((always_inline)); inline static size_t estimate_size(int64_t &new_int_size, bool is_vector); inline static array_inner *create(int64_t new_int_size, bool is_vector); @@ -176,7 +176,7 @@ private: bool is_vector_internal_or_last_index(int64_t key) const noexcept; bool has_no_string_keys() const noexcept; - size_t estimate_memory_usage() const; + size_t estimate_memory_usage() const noexcept; inline array_inner(const array_inner &other) = delete; inline array_inner &operator=(const array_inner &other) = delete; @@ -417,7 +417,7 @@ public: void reserve(int64_t int_size, bool make_vector_if_possible); - size_t estimate_memory_usage() const; + size_t estimate_memory_usage() const noexcept; template static array convert_from(const array &); diff --git a/tests/phpt/memory_usage/10_arrays.php b/tests/phpt/memory_usage/10_arrays.php new file mode 100644 index 0000000000..b90f6a772d --- /dev/null +++ b/tests/phpt/memory_usage/10_arrays.php @@ -0,0 +1,43 @@ +@ok + Date: Fri, 15 Sep 2023 14:22:59 +0300 Subject: [PATCH 2/2] distinguished `estimate_memory_usage` and `calculate_memory_for_copying` for arrays --- runtime/array.inl | 15 +++++++++++++++ runtime/array_decl.inl | 2 ++ runtime/instance-copy-processor.cpp | 2 ++ runtime/instance-copy-processor.h | 2 +- 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/runtime/array.inl b/runtime/array.inl index 15c92ed4f0..5eb7c895c5 100644 --- a/runtime/array.inl +++ b/runtime/array.inl @@ -615,6 +615,16 @@ size_t array::array_inner::estimate_memory_usage() const noexcept { return is_vector() ? sizeof_vector(buf_size) : sizeof_map(buf_size); } +template +size_t array::array_inner::calculate_memory_for_copying() const noexcept { + int64_t int_elements = size; + const bool vector_structure = is_vector(); + if (vector_structure) { + return estimate_size(int_elements, vector_structure); + } + return estimate_size(++int_elements, vector_structure); +} + template bool array::is_vector() const { return p->is_vector(); @@ -780,6 +790,11 @@ size_t array::estimate_memory_usage() const noexcept { return p->estimate_memory_usage(); } +template +size_t array::calculate_memory_for_copying() const noexcept { + return p->calculate_memory_for_copying(); +} + template typename array::const_iterator array::cbegin() const { return const_iterator::make_begin(*this); diff --git a/runtime/array_decl.inl b/runtime/array_decl.inl index 01f1ba5bff..b74921bd16 100644 --- a/runtime/array_decl.inl +++ b/runtime/array_decl.inl @@ -177,6 +177,7 @@ private: bool has_no_string_keys() const noexcept; size_t estimate_memory_usage() const noexcept; + size_t calculate_memory_for_copying() const noexcept; inline array_inner(const array_inner &other) = delete; inline array_inner &operator=(const array_inner &other) = delete; @@ -418,6 +419,7 @@ public: void reserve(int64_t int_size, bool make_vector_if_possible); size_t estimate_memory_usage() const noexcept; + size_t calculate_memory_for_copying() const noexcept; template static array convert_from(const array &); diff --git a/runtime/instance-copy-processor.cpp b/runtime/instance-copy-processor.cpp index 5ddd7c2fd6..25024adf51 100644 --- a/runtime/instance-copy-processor.cpp +++ b/runtime/instance-copy-processor.cpp @@ -20,6 +20,8 @@ bool InstanceDeepCopyVisitor::process(string &str) noexcept { return true; } + // TODO: it seems we can use `str.estimate_memory_usage(str.size())` here, + // because we don't need to take capacity into the account, only size. if (unlikely(!is_enough_memory_for(str.estimate_memory_usage()))) { str = string(); memory_limit_exceeded_ = true; diff --git a/runtime/instance-copy-processor.h b/runtime/instance-copy-processor.h index 9d5b61ea71..696e74687b 100644 --- a/runtime/instance-copy-processor.h +++ b/runtime/instance-copy-processor.h @@ -249,7 +249,7 @@ class InstanceDeepCopyVisitor : impl_::InstanceDeepBasicVisitor(); memory_limit_exceeded_ = true; return false;