Postponing copying of not-configurable PROPERTY_HANDLER properties.

Since df385232d2af a shared property is copied to object hash on the
first access. It simplified changing of configurable shared properties.

Since 50fded8ccee5 Array.prototype functions treat empty array hash as a
sign that array instance is simple (without property getters and
non-numerical properties) to iterate over array values optimally.

Accessing "length" property made a copy in array hash. As a result next
iterations over array became not optimized.

The fix is to postpone making a mutable copy of not-configurable
NJS_PROPERTY_HANDLER properties until they are really required to change.

This closes #220 issue on Github.

diff --git a/src/njs_object.h b/src/njs_object.h
index 564cced192fd04f0077d4e0f0b4c8f9a5702e6cf..e3e79cf73fb05783e2405cddcb6b4e33d537c760 100644 (file)
--- a/src/njs_object.h
+++ b/src/njs_object.h
@@ -71,6 +71,7 @@ njs_int_t njs_object_prototype_to_string(njs_vm_t *vm, njs_value_t *args,
     njs_uint_t nargs, njs_index_t unused);
 njs_int_t njs_object_length(njs_vm_t *vm, njs_value_t *value, uint32_t *dest);
 
+njs_int_t njs_prop_private_copy(njs_vm_t *vm, njs_property_query_t *pq);
 njs_object_prop_t *njs_object_prop_alloc(njs_vm_t *vm, const njs_value_t *name,
     const njs_value_t *value, uint8_t attributes);
 njs_int_t njs_object_property(njs_vm_t *vm, const njs_value_t *value,

diff --git a/src/njs_object_prop.c b/src/njs_object_prop.c
index 0152440811fa3a63b56238d5e317ac515abca4e9..b272f1b69f4933f4b153756082d9de0403e6cf60 100644 (file)
--- a/src/njs_object_prop.c
+++ b/src/njs_object_prop.c
@@ -241,6 +241,20 @@ njs_object_prop_define(njs_vm_t *vm, njs_value_t *object,
         {
             goto exception;
         }
+
+        if (pq.shared) {
+            /*
+             * shared non-configurable NJS_PROPERTY_HANDLER are not copied
+             * by njs_object_property_query().
+             */
+
+            ret = njs_prop_private_copy(vm, &pq);
+            if (njs_slow_path(ret != NJS_OK)) {
+                return ret;
+            }
+
+            prev = pq.lhq.value;
+        }
     }
 
     if (njs_is_generic_descriptor(prop)) {
@@ -358,6 +372,76 @@ exception:
 }
 
 
+njs_int_t
+njs_prop_private_copy(njs_vm_t *vm, njs_property_query_t *pq)
+{
+    njs_int_t           ret;
+    njs_function_t      *function;
+    njs_object_prop_t   *prop, *shared, *name;
+    njs_lvlhsh_query_t  lhq;
+
+    static const njs_value_t  name_string = njs_string("name");
+
+    prop = njs_mp_align(vm->mem_pool, sizeof(njs_value_t),
+                        sizeof(njs_object_prop_t));
+    if (njs_slow_path(prop == NULL)) {
+        njs_memory_error(vm);
+        return NJS_ERROR;
+    }
+
+    shared = pq->lhq.value;
+    *prop = *shared;
+
+    pq->lhq.replace = 0;
+    pq->lhq.value = prop;
+    pq->lhq.pool = vm->mem_pool;
+
+    ret = njs_lvlhsh_insert(&pq->prototype->hash, &pq->lhq);
+    if (njs_slow_path(ret != NJS_OK)) {
+        njs_internal_error(vm, "lvlhsh insert failed");
+        return NJS_ERROR;
+    }
+
+    if (!njs_is_function(&prop->value)) {
+        return NJS_OK;
+    }
+
+    function = njs_function_value_copy(vm, &prop->value);
+    if (njs_slow_path(function == NULL)) {
+        return NJS_ERROR;
+    }
+
+    if (function->ctor) {
+        function->object.shared_hash = vm->shared->function_instance_hash;
+
+    } else {
+        function->object.shared_hash = vm->shared->arrow_instance_hash;
+    }
+
+    name = njs_object_prop_alloc(vm, &name_string, &prop->name, 0);
+    if (njs_slow_path(name == NULL)) {
+        return NJS_ERROR;
+    }
+
+    name->configurable = 1;
+
+    lhq.key_hash = NJS_NAME_HASH;
+    lhq.key = njs_str_value("name");
+    lhq.replace = 0;
+    lhq.value = name;
+    lhq.proto = &njs_object_hash_proto;
+    lhq.pool = vm->mem_pool;
+
+    ret = njs_lvlhsh_insert(&function->object.hash, &lhq);
+    if (njs_slow_path(ret != NJS_OK)) {
+        njs_internal_error(vm, "lvlhsh insert failed");
+        return NJS_ERROR;
+    }
+
+    return NJS_OK;
+}
+
+
 static njs_int_t
 njs_descriptor_prop(njs_vm_t *vm, njs_object_prop_t *prop,
     const njs_value_t *desc)

diff --git a/src/njs_value.c b/src/njs_value.c
index 667a648a7e5366dee464cce97ac5dd10cc0906a8..147a12485153f06ff52eed637e24dcb76de3aa11 100644 (file)
--- a/src/njs_value.c
+++ b/src/njs_value.c
@@ -11,7 +11,6 @@
 static njs_int_t njs_object_property_query(njs_vm_t *vm,
     njs_property_query_t *pq, njs_object_t *object,
     const njs_value_t *key);
-static njs_int_t njs_prop_private_copy(njs_vm_t *vm, njs_property_query_t *pq);
 static njs_int_t njs_array_property_query(njs_vm_t *vm,
     njs_property_query_t *pq, njs_array_t *array, uint32_t index);
 static njs_int_t njs_string_property_query(njs_vm_t *vm,
@@ -681,6 +680,16 @@ njs_object_property_query(njs_vm_t *vm, njs_property_query_t *pq,
             ret = njs_lvlhsh_find(&proto->shared_hash, &pq->lhq);
 
             if (ret == NJS_OK) {
+                prop = pq->lhq.value;
+
+                if (!prop->configurable
+                    && prop->type == NJS_PROPERTY_HANDLER)
+                {
+                    /* Postpone making a mutable NJS_PROPERTY_HANDLER copy. */
+                    pq->shared = 1;
+                    return ret;
+                }
+
                 return njs_prop_private_copy(vm, pq);
             }
         }
@@ -698,76 +707,6 @@ njs_object_property_query(njs_vm_t *vm, njs_property_query_t *pq,
 }
 
 
-static njs_int_t
-njs_prop_private_copy(njs_vm_t *vm, njs_property_query_t *pq)
-{
-    njs_int_t           ret;
-    njs_function_t      *function;
-    njs_object_prop_t   *prop, *shared, *name;
-    njs_lvlhsh_query_t  lhq;
-
-    static const njs_value_t  name_string = njs_string("name");
-
-    prop = njs_mp_align(vm->mem_pool, sizeof(njs_value_t),
-                        sizeof(njs_object_prop_t));
-    if (njs_slow_path(prop == NULL)) {
-        njs_memory_error(vm);
-        return NJS_ERROR;
-    }
-
-    shared = pq->lhq.value;
-    *prop = *shared;
-
-    pq->lhq.replace = 0;
-    pq->lhq.value = prop;
-    pq->lhq.pool = vm->mem_pool;
-
-    ret = njs_lvlhsh_insert(&pq->prototype->hash, &pq->lhq);
-    if (njs_slow_path(ret != NJS_OK)) {
-        njs_internal_error(vm, "lvlhsh insert failed");
-        return NJS_ERROR;
-    }
-
-    if (!njs_is_function(&prop->value)) {
-        return NJS_OK;
-    }
-
-    function = njs_function_value_copy(vm, &prop->value);
-    if (njs_slow_path(function == NULL)) {
-        return NJS_ERROR;
-    }
-
-    if (function->ctor) {
-        function->object.shared_hash = vm->shared->function_instance_hash;
-
-    } else {
-        function->object.shared_hash = vm->shared->arrow_instance_hash;
-    }
-
-    name = njs_object_prop_alloc(vm, &name_string, &prop->name, 0);
-    if (njs_slow_path(name == NULL)) {
-        return NJS_ERROR;
-    }
-
-    name->configurable = 1;
-
-    lhq.key_hash = NJS_NAME_HASH;
-    lhq.key = njs_str_value("name");
-    lhq.replace = 0;
-    lhq.value = name;
-    lhq.proto = &njs_object_hash_proto;
-    lhq.pool = vm->mem_pool;
-
-    ret = njs_lvlhsh_insert(&function->object.hash, &lhq);
-    if (njs_slow_path(ret != NJS_OK)) {
-        njs_internal_error(vm, "lvlhsh insert failed");
-        return NJS_ERROR;
-    }
-
-    return NJS_OK;
-}
-
-
 static njs_int_t
 njs_array_property_query(njs_vm_t *vm, njs_property_query_t *pq,
     njs_array_t *array, uint32_t index)

diff --git a/src/njs_value.h b/src/njs_value.h
index 0c4c425ba8d8ea5c9a42345b0dec8a14e677d430..2144d082055d40950db2dc602a29b2e02ca2b7d1 100644 (file)
--- a/src/njs_value.h
+++ b/src/njs_value.h
@@ -352,6 +352,7 @@ typedef struct {
     njs_object_t                *prototype;
     njs_object_prop_t           *own_whiteout;
     uint8_t                     query;
+    uint8_t                     shared;
     uint8_t                     own;
 } njs_property_query_t;
 
@@ -810,6 +811,7 @@ njs_set_object_value(njs_value_t *value, njs_object_value_t *object_value)
         (pq)->lhq.value = NULL;                                               \
         (pq)->own_whiteout = NULL;                                            \
         (pq)->query = _query;                                                 \
+        (pq)->shared = 0;                                                     \
         (pq)->own = _own;                                                     \
     } while (0)
 

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 2c9749979ded83248497f043ca260427ad15e98d..d65a53e0db8b088af36ba9f7850ba697d082af51 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -3729,6 +3729,14 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("var a = [1,2]; a[100] = 100; a[100] +' '+ a.length"),
       njs_str("100 101") },
 
+    { njs_str("var a = []; Object.defineProperty(a, 'length', {writable:0});"
+              "Object.getOwnPropertyDescriptor(a, 'length').writable"),
+      njs_str("false") },
+
+    { njs_str("var a = []; Object.defineProperty(a, 'length', {writable:0});"
+              "Object.defineProperty(a, 'length', {writable:true})"),
+      njs_str("TypeError: Cannot redefine property: \"length\"") },
+
     { njs_str("Array.prototype.slice(1)"),
       njs_str("") },