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

@@ -129,7 +129,7 @@ uint32_t array<T>::array_inner::choose_bucket(const array_inner_fields_for_map &
 }
 
 template<class T>
-bool array<T>::array_inner::is_vector() const {
+bool array<T>::array_inner::is_vector() const noexcept {
   return is_vector_internal;
 }
 
@@ -203,12 +203,12 @@ const typename array<T>::array_inner_fields_for_map &array<T>::array_inner::fiel
 }
 
 template<class T>
-size_t array<T>::array_inner::sizeof_vector(uint32_t int_size) {
+size_t array<T>::array_inner::sizeof_vector(uint32_t int_size) noexcept {
   return sizeof(array_inner) + int_size * sizeof(T);
 }
 
 template<class T>
-size_t array<T>::array_inner::sizeof_map(uint32_t int_size) {
+size_t array<T>::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<T>::array_inner::has_no_string_keys() const noexcept {
 }
 
 template<class T>
-size_t array<T>::array_inner::estimate_memory_usage() const {
+size_t array<T>::array_inner::estimate_memory_usage() const noexcept {
+  return is_vector() ? sizeof_vector(buf_size) : sizeof_map(buf_size);
+}
+
+template<class T>
+size_t array<T>::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<T>::reserve(int64_t int_size, bool make_vector_if_possible) {
 }
 
 template<class T>
-size_t array<T>::estimate_memory_usage() const {
+size_t array<T>::estimate_memory_usage() const noexcept {
   return p->estimate_memory_usage();
 }
 
+template<class T>
+size_t array<T>::calculate_memory_for_copying() const noexcept {
+  return p->calculate_memory_for_copying();
+}
+
 template<class T>
 typename array<T>::const_iterator array<T>::cbegin() const {
   return const_iterator::make_begin(*this);

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<typename U>
   static array<T> convert_from(const array<U> &);

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;

runtime/instance-copy-processor.h

@@ -249,7 +249,7 @@ class InstanceDeepCopyVisitor : impl_::InstanceDeepBasicVisitor<InstanceDeepCopy
     if (arr.is_reference_counter(ExtraRefCnt::for_global_const)) {
       return true;
     }
-    if (unlikely(!is_enough_memory_for(arr.estimate_memory_usage()))) {
+    if (unlikely(!is_enough_memory_for(arr.calculate_memory_for_copying()))) {
       arr = array<T>();
       memory_limit_exceeded_ = true;
       return false;

tests/phpt/memory_usage/10_arrays.php

@@ -0,0 +1,43 @@
+@ok
+<?php
+
+function test_vector() {
+#ifndef KPHP
+  // sizeof(array_inner_control) + buf_size * sizeof(int64_t)
+  var_dump(32 + 2 * 8);
+  var_dump(32 + 2 * 8);
+  var_dump(32 + 4 * 8);
+  var_dump(32 + 4 * 8);
+  var_dump(32 + 8 * 8);
+  var_dump(32 + 8 * 8);
+  var_dump(32 + 8 * 8);
+  var_dump(32 + 8 * 8);
+  var_dump(32 + 16 * 8);
+  var_dump(32 + 16 * 8);
+  return;
+#endif
+  $v = [];
+  for ($i = 0; $i < 10; ++$i) {
+    $v[] = 42;
+    var_dump(estimate_memory_usage($v));
+  }
+}
+
+function test_map() {
+#ifndef KPHP
+  // sizeof(array_inner_fields_for_map) + sizeof(array_inner_control) + buf_size * sizeof(array_bucket)
+  var_dump(16 + 32 + 11 * 32);
+  var_dump(16 + 32 + 33 * 32);
+  return;
+#endif
+  $m = [];
+  for ($i = 0; $i < 7; ++$i) {
+    $m[$i + 1] = 42;
+  }
+  var_dump(estimate_memory_usage($m));
+  $m[23] = 42;
+  var_dump(estimate_memory_usage($m));
+}
+
+test_vector();
+test_map();