[njs] Simplified njs_property_query() API by eliminating pq->shared.

Dmitry Volyntsev xeioex at nginx.com
Tue Aug 6 16:46:08 UTC 2019


details:   https://hg.nginx.org/njs/rev/df385232d2af
branches:  
changeset: 1110:df385232d2af
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Tue Aug 06 18:46:50 2019 +0300
description:
Simplified njs_property_query() API by eliminating pq->shared.

Making copy of shared property in object's hash at the first access. It
simplifies njs_property_query() API and speeds up next access to the
property.

diffstat:

 src/njs_builtin.c     |   4 +-
 src/njs_function.c    |   2 -
 src/njs_object.c      |   8 ++--
 src/njs_object.h      |   1 -
 src/njs_object_prop.c |  92 +--------------------------------------------------
 src/njs_value.c       |  92 +++++++++++++++++++++++++++++++++++++++-----------
 src/njs_value.h       |   2 -
 src/njs_vmcode.c      |  43 ++++++-----------------
 8 files changed, 90 insertions(+), 154 deletions(-)

diffs (416 lines):

diff -r 9dab7c4514e3 -r df385232d2af src/njs_builtin.c
--- a/src/njs_builtin.c	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_builtin.c	Tue Aug 06 18:46:50 2019 +0300
@@ -1172,7 +1172,7 @@ njs_process_object_argv(njs_vm_t *vm, nj
     lhq.value = prop;
     lhq.key_hash = NJS_ARGV_HASH;
     lhq.key = njs_str_value("argv");
-    lhq.replace = 0;
+    lhq.replace = 1;
     lhq.pool = vm->mem_pool;
     lhq.proto = &njs_object_hash_proto;
 
@@ -1270,7 +1270,7 @@ njs_process_object_env(njs_vm_t *vm, njs
 
     njs_set_object(&prop->value, env);
 
-    lhq.replace = 0;
+    lhq.replace = 1;
     lhq.pool = vm->mem_pool;
     lhq.proto = &njs_object_hash_proto;
     lhq.value = prop;
diff -r 9dab7c4514e3 -r df385232d2af src/njs_function.c
--- a/src/njs_function.c	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_function.c	Tue Aug 06 18:46:50 2019 +0300
@@ -815,13 +815,11 @@ njs_function_property_prototype_create(n
     njs_function_t  *function;
 
     prototype = njs_object_alloc(vm);
-
     if (njs_slow_path(prototype == NULL)) {
         return NULL;
     }
 
     function = njs_function_value_copy(vm, value);
-
     if (njs_slow_path(function == NULL)) {
         return NULL;
     }
diff -r 9dab7c4514e3 -r df385232d2af src/njs_object.c
--- a/src/njs_object.c	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_object.c	Tue Aug 06 18:46:50 2019 +0300
@@ -1611,9 +1611,9 @@ njs_value_t *
 njs_property_prototype_create(njs_vm_t *vm, njs_lvlhsh_t *hash,
     njs_object_t *prototype)
 {
-    njs_int_t                 ret;
-    njs_object_prop_t         *prop;
-    njs_lvlhsh_query_t        lhq;
+    njs_int_t           ret;
+    njs_object_prop_t   *prop;
+    njs_lvlhsh_query_t  lhq;
 
     static const njs_value_t  proto_string = njs_string("prototype");
 
@@ -1629,7 +1629,7 @@ njs_property_prototype_create(njs_vm_t *
     lhq.value = prop;
     lhq.key_hash = NJS_PROTOTYPE_HASH;
     lhq.key = njs_str_value("prototype");
-    lhq.replace = 0;
+    lhq.replace = 1;
     lhq.pool = vm->mem_pool;
     lhq.proto = &njs_object_hash_proto;
 
diff -r 9dab7c4514e3 -r df385232d2af src/njs_object.h
--- a/src/njs_object.h	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_object.h	Tue Aug 06 18:46:50 2019 +0300
@@ -62,7 +62,6 @@ njs_int_t njs_object_prop_define(njs_vm_
     njs_value_t *name, njs_value_t *value);
 njs_int_t njs_object_prop_descriptor(njs_vm_t *vm, njs_value_t *dest,
     njs_value_t *value, njs_value_t *setval);
-njs_int_t njs_prop_private_copy(njs_vm_t *vm, njs_property_query_t *pq);
 const char *njs_prop_type_string(njs_object_prop_type_t type);
 
 extern const njs_object_init_t  njs_object_constructor_init;
diff -r 9dab7c4514e3 -r df385232d2af src/njs_object_prop.c
--- a/src/njs_object_prop.c	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_object_prop.c	Tue Aug 06 18:46:50 2019 +0300
@@ -156,14 +156,6 @@ njs_object_prop_define(njs_vm_t *vm, njs
 
     /* Updating existing prop. */
 
-    if (njs_slow_path(pq.shared)) {
-        ret = njs_prop_private_copy(vm, &pq);
-
-        if (njs_slow_path(ret != NJS_OK)) {
-            return ret;
-        }
-    }
-
     prev = pq.lhq.value;
 
     switch (prev->type) {
@@ -458,6 +450,7 @@ njs_object_prop_descriptor(njs_vm_t *vm,
     prop = pq.lhq.value;
 
     switch (prop->type) {
+    case NJS_METHOD:
     case NJS_PROPERTY:
         break;
 
@@ -471,19 +464,6 @@ njs_object_prop_descriptor(njs_vm_t *vm,
 
         break;
 
-    case NJS_METHOD:
-        if (pq.shared) {
-            ret = njs_prop_private_copy(vm, &pq);
-
-            if (njs_slow_path(ret != NJS_OK)) {
-                return ret;
-            }
-
-            prop = pq.lhq.value;
-        }
-
-        break;
-
     default:
         njs_type_error(vm, "unexpected property type: %s",
                        njs_prop_type_string(prop->type));
@@ -615,76 +595,6 @@ njs_object_prop_descriptor(njs_vm_t *vm,
 }
 
 
-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;
-}
-
-
 const char *
 njs_prop_type_string(njs_object_prop_type_t type)
 {
diff -r 9dab7c4514e3 -r df385232d2af src/njs_value.c
--- a/src/njs_value.c	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_value.c	Tue Aug 06 18:46:50 2019 +0300
@@ -11,6 +11,7 @@
 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,
@@ -659,9 +660,7 @@ njs_object_property_query(njs_vm_t *vm, 
             ret = njs_lvlhsh_find(&proto->shared_hash, &pq->lhq);
 
             if (ret == NJS_OK) {
-                pq->shared = 1;
-
-                return ret;
+                return njs_prop_private_copy(vm, pq);
             }
         }
 
@@ -679,6 +678,76 @@ 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)
 {
@@ -931,18 +1000,6 @@ njs_value_property(njs_vm_t *vm, njs_val
         switch (prop->type) {
 
         case NJS_METHOD:
-            if (pq.shared) {
-                ret = njs_prop_private_copy(vm, &pq);
-
-                if (njs_slow_path(ret != NJS_OK)) {
-                    return ret;
-                }
-
-                prop = pq.lhq.value;
-            }
-
-            /* Fall through. */
-
         case NJS_PROPERTY:
             if (njs_is_data_descriptor(prop)) {
                 *retval = prop->value;
@@ -1052,11 +1109,6 @@ njs_value_property_set(njs_vm_t *vm, njs
             switch (prop->type) {
             case NJS_PROPERTY:
             case NJS_METHOD:
-                if (njs_slow_path(pq.shared)) {
-                    shared = prop;
-                    break;
-                }
-
                 goto found;
 
             case NJS_PROPERTY_REF:
diff -r 9dab7c4514e3 -r df385232d2af src/njs_value.h
--- a/src/njs_value.h	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_value.h	Tue Aug 06 18:46:50 2019 +0300
@@ -352,7 +352,6 @@ 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;
 
@@ -799,7 +798,6 @@ 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 9dab7c4514e3 -r df385232d2af src/njs_vmcode.c
--- a/src/njs_vmcode.c	Tue Aug 06 17:58:37 2019 +0300
+++ b/src/njs_vmcode.c	Tue Aug 06 18:46:50 2019 +0300
@@ -1224,7 +1224,7 @@ static njs_jump_off_t
 njs_vmcode_property_delete(njs_vm_t *vm, njs_value_t *value, njs_value_t *key)
 {
     njs_jump_off_t        ret;
-    njs_object_prop_t     *prop, *whipeout;
+    njs_object_prop_t     *prop;
     njs_property_query_t  pq;
 
     njs_property_query_init(&pq, NJS_PROPERTY_QUERY_DELETE, 1);
@@ -1242,32 +1242,19 @@ njs_vmcode_property_delete(njs_vm_t *vm,
             return NJS_ERROR;
         }
 
-        if (njs_slow_path(pq.shared)) {
-            whipeout = njs_mp_align(vm->mem_pool, sizeof(njs_value_t),
-                                    sizeof(njs_object_prop_t));
-            if (njs_slow_path(whipeout == NULL)) {
-                njs_memory_error(vm);
-                return NJS_ERROR;
+        switch (prop->type) {
+        case NJS_PROPERTY_HANDLER:
+            if (njs_is_external(value)) {
+                ret = prop->value.data.u.prop_handler(vm, value, NULL, NULL);
+                if (njs_slow_path(ret != NJS_OK)) {
+                    return ret;
+                }
+
+                goto done;
             }
 
-            njs_set_invalid(&whipeout->value);
-            whipeout->name = prop->name;
-            whipeout->type = NJS_WHITEOUT;
-
-            pq.lhq.replace = 0;
-            pq.lhq.value = whipeout;
-            pq.lhq.pool = vm->mem_pool;
+            /* Fall through. */
 
-            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;
-            }
-
-            break;
-        }
-
-        switch (prop->type) {
         case NJS_PROPERTY:
         case NJS_METHOD:
             break;
@@ -1276,14 +1263,6 @@ njs_vmcode_property_delete(njs_vm_t *vm,
             njs_set_invalid(prop->value.data.u.value);
             goto done;
 
-        case NJS_PROPERTY_HANDLER:
-            ret = prop->value.data.u.prop_handler(vm, value, NULL, NULL);
-            if (njs_slow_path(ret != NJS_OK)) {
-                return ret;
-            }
-
-            goto done;
-
         default:
             njs_internal_error(vm, "unexpected property type \"%s\" "
                                "while deleting",


More information about the nginx-devel mailing list