From afdf709127ee964692c8792f98c9f701e5c24385 Mon Sep 17 00:00:00 2001 From: Stanislav Eismont Date: Fri, 15 Sep 2023 18:15:13 +0300 Subject: [PATCH] fix estimate_memory_usage() function (#900) - Use capacity instead of size in estimate_memory_usage() - Extract calculate_memory_for_copying() which uses size (old estimate_memory_usage()) --- runtime/array.inl | 20 +++++++++---- runtime/array_decl.inl | 12 ++++---- runtime/instance-copy-processor.cpp | 2 ++ runtime/instance-copy-processor.h | 2 +- tests/phpt/memory_usage/10_arrays.php | 43 +++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 tests/phpt/memory_usage/10_arrays.php diff --git a/runtime/array.inl b/runtime/array.inl index b0830a8dd4..5eb7c895c5 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,7 +611,12 @@ bool array::array_inner::has_no_string_keys() const noexcept { } template -size_t array::array_inner::estimate_memory_usage() const { +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) { @@ -781,10 +786,15 @@ 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(); } +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 16ad2c61a4..b74921bd16 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,8 @@ 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; + 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; @@ -417,7 +418,8 @@ 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; + 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; 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 +