Replace per-VM indexed array for modules with hash-based lookup.

Previously, evaluated module values were cached in a separate
vm->modules array indexed by module->index assigned at compile time.
This required keeping the array size in sync with shared->module_items,
which was error-prone in interactive mode where new modules could be
compiled across commands.

Instead, store evaluated module values directly in the per-VM module
copy already maintained by vm->modules_hash.  This unifies the import
and require() caching paths and eliminates the index-based array along
with module->index and shared->module_items fields.

This fixes `make shell_test` when configured with --debug=YES.

diff --git a/src/njs_module.c b/src/njs_module.c
index b971035a3f8b868fcd40276b7a782f7f9d3f2fe3..a16134390dbb982df8bb1e1c5c5e1811323837ba 100644 (file)
--- a/src/njs_module.c
+++ b/src/njs_module.c
@@ -66,9 +66,11 @@ njs_module_find(njs_vm_t *vm, njs_str_t *name, njs_bool_t shared)
 
         memcpy(module, shrd, sizeof(njs_mod_t));
 
-        object = njs_object_value_copy(vm, &module->value);
-        if (njs_slow_path(object == NULL)) {
-            return NULL;
+        if (module->function.native) {
+            object = njs_object_value_copy(vm, &module->value);
+            if (njs_slow_path(object == NULL)) {
+                return NULL;
+            }
         }
 
         fhq.replace = 0;
@@ -76,6 +78,7 @@ njs_module_find(njs_vm_t *vm, njs_str_t *name, njs_bool_t shared)
 
         ret = njs_flathsh_insert(&vm->modules_hash, &fhq);
         if (njs_slow_path(ret != NJS_OK)) {
+            njs_internal_error(vm, "flathsh insert failed");
             return NULL;
         }
 

diff --git a/src/njs_module.h b/src/njs_module.h
index 4510a9b07617f2ca89cd8596eb37ade5959e2420..f04fdac03a2450d2f72d561264f63e93b593c939 100644 (file)
--- a/src/njs_module.h
+++ b/src/njs_module.h
@@ -11,7 +11,6 @@
 struct njs_mod_s {
     njs_str_t                   name;
     njs_value_t                 value;
-    njs_index_t                 index;
     njs_function_t              function;
 };
 

diff --git a/src/njs_parser.c b/src/njs_parser.c
index 60fa26934606b9d3b05211c20b17814f7257cad3..e2d6f388d4c627ed151c37b996be5195088c7a4d 100644 (file)
--- a/src/njs_parser.c
+++ b/src/njs_parser.c
@@ -8125,10 +8125,6 @@ njs_parser_module(njs_parser_t *parser, njs_str_t *name)
 
 done:
 
-    if (module->index == 0) {
-        module->index = vm->shared->module_items++;
-    }
-
     return module;
 }
 

diff --git a/src/njs_vm.c b/src/njs_vm.c
index a9564241f643b48aff0613a991bd089a1bc24798..0bd37ae384eea1fe289cd2195caa2720ff602a1b 100644 (file)
--- a/src/njs_vm.c
+++ b/src/njs_vm.c
@@ -382,7 +382,7 @@ njs_vm_reuse(njs_vm_t *vm)
 {
     vm->active_frame = NULL;
     vm->top_frame = NULL;
-    vm->modules = NULL;
+    njs_flathsh_init(&vm->modules_hash);
 
     return njs_object_make_shared(vm, njs_object(&vm->global_value));
 }

diff --git a/src/njs_vm.h b/src/njs_vm.h
index 4329c926997b2ec5e12f05086dfe37a304a4f70a..bb9e9c39e74245248abfd602557e6d6611ca5d52 100644 (file)
--- a/src/njs_vm.h
+++ b/src/njs_vm.h
@@ -136,7 +136,6 @@ struct njs_vm_s {
 
     njs_flathsh_t            values_hash;
 
-    njs_arr_t                *modules;
     njs_flathsh_t            modules_hash;
 
     uint32_t                 event_id;
@@ -216,7 +215,6 @@ struct njs_vm_shared_s {
     njs_flathsh_t            arguments_object_instance_hash;
     njs_flathsh_t            regexp_instance_hash;
 
-    size_t                   module_items;
     njs_flathsh_t            modules_hash;
 
     njs_flathsh_t            env_hash;

diff --git a/src/njs_vmcode.c b/src/njs_vmcode.c
index 2dac79e6c13480fc0a3a5fba277e3ce976d62809..c89e7f12f03b10ed4c36b201d6d05282077f367f 100644 (file)
--- a/src/njs_vmcode.c
+++ b/src/njs_vmcode.c
@@ -2666,55 +2666,27 @@ njs_vmcode_return(njs_vm_t *vm, njs_value_t *dst, njs_value_t *retval)
 static njs_jump_off_t
 njs_vmcode_import(njs_vm_t *vm, njs_mod_t *module, njs_value_t *retval)
 {
-    njs_int_t     ret;
-    njs_arr_t     *m;
-    njs_value_t   *value;
-    njs_object_t  *object;
-
-    if (vm->modules == NULL) {
-        vm->modules = njs_arr_create(vm->mem_pool, 4, sizeof(njs_value_t));
-        if (njs_slow_path(vm->modules == NULL)) {
-            njs_memory_error(vm);
-            return NJS_ERROR;
-        }
-
-        m = vm->modules;
-
-        value = njs_arr_add_multiple(m, vm->shared->module_items);
-        if (njs_slow_path(value == NULL)) {
-            njs_memory_error(vm);
-            return NJS_ERROR;
-        }
+    njs_int_t  ret;
+    njs_mod_t  *m;
 
-        njs_memzero(m->start, m->items * sizeof(njs_value_t));
+    m = njs_module_find(vm, &module->name, 0);
+    if (njs_slow_path(m == NULL)) {
+        return NJS_ERROR;
     }
 
-    njs_assert(module->index < vm->modules->items);
-
-    value = njs_arr_item(vm->modules, module->index);
-
-    if (!njs_is_null(value)) {
-        njs_value_assign(retval, value);
+    if (!njs_is_null(&m->value)) {
+        njs_value_assign(retval, &m->value);
         return sizeof(njs_vmcode_import_t);
     }
 
-    if (module->function.native) {
-        njs_value_assign(value, &module->value);
-
-        object = njs_object_value_copy(vm, value);
-        if (njs_slow_path(object == NULL)) {
-            return NJS_ERROR;
-        }
+    njs_set_invalid(&m->value);
 
-    } else {
-        njs_set_invalid(value);
-        ret = njs_vm_invoke(vm, &module->function, NULL, 0, value);
-        if (ret == NJS_ERROR) {
-            return ret;
-        }
+    ret = njs_vm_invoke(vm, &m->function, NULL, 0, &m->value);
+    if (ret == NJS_ERROR) {
+        return ret;
     }
 
-    njs_value_assign(retval, value);
+    njs_value_assign(retval, &m->value);
 
     return sizeof(njs_vmcode_import_t);
 }

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 806e771e94058407c3520f67212f2caf4a296a3b..59f95e34b10913448e6f365834a553a2512f2169 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -21062,12 +21062,20 @@ static njs_unit_test_t  njs_async_handler_test[] =
 
 static njs_unit_test_t  njs_shared_test[] =
 {
+    { njs_str("var cr = require('unknown')"),
+      njs_str("Error: Cannot load module \"unknown\"\n"
+              "    at require (native)\n"
+              "    at main (:1)\n") },
+
     { njs_str("var cr = require('crypto'); cr.createHash"),
       njs_str("[object Function]") },
 
     { njs_str("var cr = require('crypto'); cr.createHash('md5')"),
       njs_str("[object Hash]") },
 
+    { njs_str("import cr from 'unknown'"),
+      njs_str("Error: Cannot load module \"unknown\"") },
+
     { njs_str("import cr from 'crypto'; cr.createHash"),
       njs_str("[object Function]") },
 
@@ -21935,6 +21943,16 @@ njs_runtime_destroy(njs_runtime_t *rt)
 }
 
 
+static njs_mod_t *
+njs_unit_test_module_loader(njs_vm_t *vm, njs_external_ptr_t external,
+    njs_str_t *name)
+{
+    njs_vm_error(vm, "Cannot load module \"%V\"", name);
+
+    return NULL;
+}
+
+
 static njs_int_t
 njs_process_test(njs_external_state_t *state, njs_opts_t *opts,
     njs_unit_test_t *expected)
@@ -22110,6 +22128,8 @@ njs_unit_test(njs_unit_test_t tests[], size_t num, njs_str_t *name,
             goto done;
         }
 
+        njs_vm_set_module_loader(vm, njs_unit_test_module_loader, NULL);
+
         if (opts->preload) {
             start = preload.start;
             end = start + preload.length;