Rework property hashmap delete. (#1543)

Triggering hashmap recreate during property delete may damage the
property chain list, so recreate postponed after the delete is done.

Next attempt to fix #1533.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com

4 files changed, 68 insertions, 28 deletions

diff --git a/jerry-core/ecma/base/ecma-helpers.c b/jerry-core/ecma/base/ecma-helpers.c
index 530fb750..43f725f7 100644
--- a/jerry-core/ecma/base/ecma-helpers.c
+++ b/jerry-core/ecma/base/ecma-helpers.c
@@ -737,7 +737,7 @@ ecma_find_named_property (ecma_object_t *obj_p, /**< object to find property in
                                     prop_iter_p->next_property_cp);
   }
 
-  if (steps > (ECMA_PROPERTY_HASMAP_MINIMUM_SIZE / 4))
+  if (steps >= (ECMA_PROPERTY_HASMAP_MINIMUM_SIZE / 2))
   {
     ecma_property_hashmap_create (obj_p);
   }
@@ -863,14 +863,14 @@ ecma_delete_property (ecma_object_t *object_p, /**< object */
 {
   ecma_property_header_t *cur_prop_p = ecma_get_property_list (object_p);
   ecma_property_header_t *prev_prop_p = NULL;
-  bool has_hashmap = false;
+  ecma_property_hashmap_delete_status hashmap_status = ECMA_PROPERTY_HASHMAP_DELETE_NO_HASHMAP;
 
   if (cur_prop_p != NULL && cur_prop_p->types[0] == ECMA_PROPERTY_TYPE_HASHMAP)
   {
     prev_prop_p = cur_prop_p;
     cur_prop_p = ECMA_GET_POINTER (ecma_property_header_t,
                                    cur_prop_p->next_property_cp);
-    has_hashmap = true;
+    hashmap_status = ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP;
   }
 
   while (true)
@@ -886,11 +886,11 @@ ecma_delete_property (ecma_object_t *object_p, /**< object */
       {
         JERRY_ASSERT (ECMA_PROPERTY_GET_TYPE (cur_prop_p->types[i]) != ECMA_PROPERTY_TYPE_SPECIAL);
 
-        if (has_hashmap)
+        if (hashmap_status == ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP)
         {
-          ecma_property_hashmap_delete (object_p,
-                                        prop_pair_p->names_cp[i],
-                                        cur_prop_p->types + i);
+          hashmap_status = ecma_property_hashmap_delete (object_p,
+                                                         prop_pair_p->names_cp[i],
+                                                         cur_prop_p->types + i);
         }
 
         ecma_free_property (object_p, prop_pair_p->names_cp[i], cur_prop_p->types + i);
@@ -915,6 +915,12 @@ ecma_delete_property (ecma_object_t *object_p, /**< object */
         }
 
         ecma_dealloc_property_pair ((ecma_property_pair_t *) cur_prop_p);
+
+        if (hashmap_status == ECMA_PROPERTY_HASHMAP_DELETE_RECREATE_HASHMAP)
+        {
+          ecma_property_hashmap_free (object_p);
+          ecma_property_hashmap_create (object_p);
+        }
         return;
       }
     }
@@ -989,14 +995,14 @@ ecma_delete_array_properties (ecma_object_t *object_p, /**< object */
   /* Second all properties between new_length and old_length are deleted. */
   current_prop_p = ecma_get_property_list (object_p);
   ecma_property_header_t *prev_prop_p = NULL;
-  bool has_hashmap = false;
+  ecma_property_hashmap_delete_status hashmap_status = ECMA_PROPERTY_HASHMAP_DELETE_NO_HASHMAP;
 
   if (current_prop_p->types[0] == ECMA_PROPERTY_TYPE_HASHMAP)
   {
     prev_prop_p = current_prop_p;
     current_prop_p = ECMA_GET_POINTER (ecma_property_header_t,
                                        current_prop_p->next_property_cp);
-    has_hashmap = true;
+    hashmap_status = ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP;
   }
 
   while (current_prop_p != NULL)
@@ -1015,9 +1021,11 @@ ecma_delete_array_properties (ecma_object_t *object_p, /**< object */
         {
           JERRY_ASSERT (index != ECMA_STRING_NOT_ARRAY_INDEX);
 
-          if (has_hashmap)
+          if (hashmap_status == ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP)
           {
-            ecma_property_hashmap_delete (object_p, prop_pair_p->names_cp[i], current_prop_p->types + i);
+            hashmap_status = ecma_property_hashmap_delete (object_p,
+                                                           prop_pair_p->names_cp[i],
+                                                           current_prop_p->types + i);
           }
 
           ecma_free_property (object_p, prop_pair_p->names_cp[i], current_prop_p->types + i);
@@ -1052,6 +1060,12 @@ ecma_delete_array_properties (ecma_object_t *object_p, /**< object */
     }
   }
 
+  if (hashmap_status == ECMA_PROPERTY_HASHMAP_DELETE_RECREATE_HASHMAP)
+  {
+    ecma_property_hashmap_free (object_p);
+    ecma_property_hashmap_create (object_p);
+  }
+
   return new_length;
 } /* ecma_delete_array_properties */
 
diff --git a/jerry-core/ecma/base/ecma-property-hashmap.c b/jerry-core/ecma/base/ecma-property-hashmap.c
index ed3b1db9..e9032a8a 100644
--- a/jerry-core/ecma/base/ecma-property-hashmap.c
+++ b/jerry-core/ecma/base/ecma-property-hashmap.c
@@ -75,18 +75,22 @@ void
 ecma_property_hashmap_create (ecma_object_t *object_p) /**< object */
 {
 #ifndef CONFIG_ECMA_PROPERTY_HASHMAP_DISABLE
-  JERRY_ASSERT (ecma_get_property_list (object_p) != NULL);
-  JERRY_ASSERT (ECMA_PROPERTY_IS_PROPERTY_PAIR (ecma_get_property_list (object_p)));
-
   if (JERRY_CONTEXT (ecma_prop_hashmap_alloc_state) != ECMA_PROP_HASHMAP_ALLOC_ON)
   {
     return;
   }
 
-  uint32_t named_property_count = 0;
-
   ecma_property_header_t *prop_iter_p = ecma_get_property_list (object_p);
 
+  if (prop_iter_p == NULL)
+  {
+    return;
+  }
+
+  JERRY_ASSERT (ECMA_PROPERTY_IS_PROPERTY_PAIR (prop_iter_p));
+
+  uint32_t named_property_count = 0;
+
   while (prop_iter_p != NULL)
   {
     JERRY_ASSERT (ECMA_PROPERTY_IS_PROPERTY_PAIR (prop_iter_p));
@@ -104,6 +108,11 @@ ecma_property_hashmap_create (ecma_object_t *object_p) /**< object */
                                     prop_iter_p->next_property_cp);
   }
 
+  if (named_property_count < (ECMA_PROPERTY_HASMAP_MINIMUM_SIZE / 2))
+  {
+    return;
+  }
+
   /* The max_property_count must be power of 2. */
   uint32_t max_property_count = ECMA_PROPERTY_HASMAP_MINIMUM_SIZE;
 
@@ -321,8 +330,11 @@ ecma_property_hashmap_insert (ecma_object_t *object_p, /**< object */
 
 /**
  * Delete named property from the hashmap.
+ *
+ * @return ECMA_PROPERTY_HASHMAP_DELETE_RECREATE_HASHMAP if hashmap should be recreated
+ *         ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP otherwise
  */
-void
+ecma_property_hashmap_delete_status
 ecma_property_hashmap_delete (ecma_object_t *object_p, /**< object */
                               jmem_cpointer_t name_cp, /**< property name */
                               ecma_property_t *property_p) /**< property */
@@ -338,15 +350,7 @@ ecma_property_hashmap_delete (ecma_object_t *object_p, /**< object */
   /* The NULLs are above 3/4 of the hashmap. */
   if (hashmap_p->null_count > ((hashmap_p->max_property_count * 3) >> 2))
   {
-    uint32_t max_property_count = hashmap_p->max_property_count;
-
-    ecma_property_hashmap_free (object_p);
-
-    if (max_property_count >= ECMA_PROPERTY_HASMAP_MINIMUM_SIZE * 2)
-    {
-      ecma_property_hashmap_create (object_p);
-    }
-    return;
+    return ECMA_PROPERTY_HASHMAP_DELETE_RECREATE_HASHMAP;
   }
 
   uint32_t entry_index = ecma_string_get_property_name_hash (*property_p, name_cp);
@@ -390,7 +394,7 @@ ecma_property_hashmap_delete (ecma_object_t *object_p, /**< object */
 
         pair_list_p[entry_index] = ECMA_NULL_POINTER;
         ECMA_PROPERTY_HASHMAP_SET_BIT (bits_p, entry_index);
-        return;
+        return ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP;
       }
     }
     else
@@ -410,6 +414,7 @@ ecma_property_hashmap_delete (ecma_object_t *object_p, /**< object */
   JERRY_UNUSED (name_cp);
   JERRY_UNUSED (property_p);
 #endif /* !CONFIG_ECMA_PROPERTY_HASHMAP_DISABLE */
+  return ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP;
 } /* ecma_property_hashmap_delete */
 
 #ifndef CONFIG_ECMA_PROPERTY_HASHMAP_DISABLE
diff --git a/jerry-core/ecma/base/ecma-property-hashmap.h b/jerry-core/ecma/base/ecma-property-hashmap.h
index d1a0f75f..3ad4dbbd 100644
--- a/jerry-core/ecma/base/ecma-property-hashmap.h
+++ b/jerry-core/ecma/base/ecma-property-hashmap.h
@@ -52,11 +52,22 @@ typedef struct
    */
 } ecma_property_hashmap_t;
 
+/**
+ * Simple ecma values
+ */
+typedef enum
+{
+  ECMA_PROPERTY_HASHMAP_DELETE_NO_HASHMAP, /**< object has no hashmap */
+  ECMA_PROPERTY_HASHMAP_DELETE_HAS_HASHMAP, /**< object has hashmap */
+  ECMA_PROPERTY_HASHMAP_DELETE_RECREATE_HASHMAP, /**< hashmap should be recreated */
+} ecma_property_hashmap_delete_status;
+
 void ecma_property_hashmap_create (ecma_object_t *object_p);
 void ecma_property_hashmap_free (ecma_object_t *object_p);
 void ecma_property_hashmap_insert (ecma_object_t *object_p, ecma_string_t *name_p,
                                    ecma_property_pair_t *property_pair_p, int property_index);
-void ecma_property_hashmap_delete (ecma_object_t *object_p, jmem_cpointer_t name_cp, ecma_property_t *property_p);
+ecma_property_hashmap_delete_status ecma_property_hashmap_delete (ecma_object_t *object_p, jmem_cpointer_t name_cp,
+                                                                  ecma_property_t *property_p);
 
 #ifndef CONFIG_ECMA_PROPERTY_HASHMAP_DISABLE
 ecma_property_t *ecma_property_hashmap_find (ecma_property_hashmap_t *hashmap_p, ecma_string_t *name_p,
diff --git a/tests/jerry/regression-test-issue-1533.js b/tests/jerry/regression-test-issue-1533.js
index 3b77ec50..b91cb231 100644
--- a/tests/jerry/regression-test-issue-1533.js
+++ b/tests/jerry/regression-test-issue-1533.js
@@ -24,3 +24,13 @@ for (var i = 0; i < 200; ++i)
   delete a[i]
 
 a[0] = 5
+
+a = [];
+
+for (var i = 0; i < 200; ++i)
+  a[i] = 5;
+
+a.length = 0
+
+for (var i = 0; i < 200; ++i)
+  a[i] = 5;