details: https://hg.nginx.org/njs/rev/ac633d007ac5 branches: changeset: 1154:ac633d007ac5 user: Dmitry Volyntsev <xei...@nginx.com> date: Thu Aug 29 19:18:53 2019 +0300 description: 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. diffstat: src/njs_object.h | 1 + src/njs_object_prop.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ src/njs_value.c | 81 +++++---------------------------------------- src/njs_value.h | 2 + src/test/njs_unit_test.c | 8 ++++ 5 files changed, 105 insertions(+), 71 deletions(-) diffs (254 lines): diff -r 8609f5316ff1 -r ac633d007ac5 src/njs_object.h --- a/src/njs_object.h Thu Aug 29 17:11:41 2019 +0300 +++ b/src/njs_object.h Thu Aug 29 19:18:53 2019 +0300 @@ -71,6 +71,7 @@ njs_int_t njs_object_prototype_to_string 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 -r 8609f5316ff1 -r ac633d007ac5 src/njs_object_prop.c --- a/src/njs_object_prop.c Thu Aug 29 17:11:41 2019 +0300 +++ b/src/njs_object_prop.c Thu Aug 29 19:18:53 2019 +0300 @@ -241,6 +241,20 @@ njs_object_prop_define(njs_vm_t *vm, njs { 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 -r 8609f5316ff1 -r ac633d007ac5 src/njs_value.c --- a/src/njs_value.c Thu Aug 29 17:11:41 2019 +0300 +++ b/src/njs_value.c Thu Aug 29 19:18:53 2019 +0300 @@ -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, 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); } } @@ -699,76 +708,6 @@ njs_object_property_query(njs_vm_t *vm, 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 -r 8609f5316ff1 -r ac633d007ac5 src/njs_value.h --- a/src/njs_value.h Thu Aug 29 17:11:41 2019 +0300 +++ b/src/njs_value.h Thu Aug 29 19:18:53 2019 +0300 @@ -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, (pq)->lhq.value = NULL; \ (pq)->own_whiteout = NULL; \ (pq)->query = _query; \ + (pq)->shared = 0; \ (pq)->own = _own; \ } while (0) diff -r 8609f5316ff1 -r ac633d007ac5 src/test/njs_unit_test.c --- a/src/test/njs_unit_test.c Thu Aug 29 17:11:41 2019 +0300 +++ b/src/test/njs_unit_test.c Thu Aug 29 19:18:53 2019 +0300 @@ -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("") }, _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel