[njs] Postponing copying of not-configurable PROPERTY_HANDLER properties.
Dmitry Volyntsev
xeioex at nginx.com
Thu Aug 29 16:19:07 UTC 2019
details: https://hg.nginx.org/njs/rev/ac633d007ac5
branches:
changeset: 1154:ac633d007ac5
user: Dmitry Volyntsev <xeioex at 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("") },
More information about the nginx-devel
mailing list