details: https://github.com/nginx/njs/commit/855aa4c9fac01bd9fbdb1602b523edc00117ff09
branches: master
commit: 855aa4c9fac01bd9fbdb1602b523edc00117ff09
user: Dmitry Volyntsev <xeioex@nginx.com>
date: Fri, 3 Jan 2025 22:25:15 -0800
description:
Modules: removed extra VM creation per server.

Previously, when js_import was declared in http or stream blocks, an extra
copy of the VM instance was created for each server block. This was not
needed and consumed a lot of memory for configurations with many server
blocks.

This issue was introduced in 9b674412 (0.8.6) and was
partially fixed for location blocks only in 685b64f0 (0.8.7).

---
nginx/ngx_js.c | 17 ++++++++
nginx/t/js_import2.t | 12 +++++-
nginx/t/js_merge_location_blocks.t | 83 ++++++++++++++++++++++++++++++++++++++
nginx/t/js_merge_server_blocks.t | 78 +++++++++++++++++++++++++++++++++++
nginx/t/stream_js.t | 6 ++-
5 files changed, 194 insertions(+), 2 deletions(-)

diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c
index 12b577a2..5c2a44cb 100644
--- a/nginx/ngx_js.c
+++ b/nginx/ngx_js.c
@@ -3343,6 +3343,16 @@ ngx_js_merge_vm(ngx_conf_t *cf, ngx_js_loc_conf_t *conf,
ngx_array_t *imports, *preload_objects, *paths;
ngx_js_named_path_t *import, *pi, *pij, *preload;

+ if (prev->imports != NGX_CONF_UNSET_PTR && prev->engine == NULL) {
+ /*
+ * special handling to preserve conf->engine
+ * in the "http" or "stream" section to inherit it to all servers
+ */
+ if (init_vm(cf, (ngx_js_loc_conf_t *) prev) != NGX_OK) {
+ return NGX_ERROR;
+ }
+ }
+
if (conf->imports == NGX_CONF_UNSET_PTR
&& conf->type == prev->type
&& conf->paths == NGX_CONF_UNSET_PTR
@@ -3851,6 +3861,9 @@ ngx_js_init_conf_vm(ngx_conf_t *cf, ngx_js_loc_conf_t *conf,
return NGX_ERROR;
}

+ ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, "js vm init %s: %p",
+ conf->engine->name, conf->engine);
+
cln = ngx_pool_cleanup_add(cf->pool, 0);
if (cln == NULL) {
return NGX_ERROR;
@@ -4039,6 +4052,10 @@ ngx_js_merge_conf(ngx_conf_t *cf, void *parent, void *child,
ngx_js_loc_conf_t *conf = child;

ngx_conf_merge_uint_value(conf->type, prev->type, NGX_ENGINE_NJS);
+ if (prev->type == NGX_CONF_UNSET_UINT) {
+ prev->type = NGX_ENGINE_NJS;
+ }
+
ngx_conf_merge_msec_value(conf->timeout, prev->timeout, 60000);
ngx_conf_merge_size_value(conf->reuse, prev->reuse, 128);
ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size, 16384);
diff --git a/nginx/t/js_import2.t b/nginx/t/js_import2.t
index cd29d2dc..7fdc624d 100644
--- a/nginx/t/js_import2.t
+++ b/nginx/t/js_import2.t
@@ -41,6 +41,7 @@ http {

js_set $test foo.bar.p;

+ # context 1
js_import foo from main.js;

location /njs {
@@ -52,11 +53,13 @@ http {
}

location /test_lib {
+ # context 2
js_import lib.js;
js_content lib.test;
}

location /test_fun {
+ # context 3
js_import fun.js;
js_content fun;
}
@@ -75,6 +78,7 @@ http {
server_name localhost;

location /test_fun {
+ # context 4
js_import fun.js;
js_content fun;
}
@@ -114,7 +118,7 @@ $t->write_file('main.js', <<EOF);

EOF

-$t->try_run('no njs available')->plan(5);
+$t->try_run('no njs available')->plan(6);

###############################################################################

@@ -124,4 +128,10 @@ like(http_get('/test_fun'), qr/FUN-TEST/s, 'fun');
like(http_get('/proxy/test_fun'), qr/FUN-TEST/s, 'proxy fun');
like(http_get('/test_var'), qr/P-TEST/s, 'foo.bar.p');

+$t->stop();
+
+my $content = $t->read_file('error.log');
+my $count = () = $content =~ m/js vm init/g;
+ok($count == 4, 'uniq js vm contexts');
+
###############################################################################
diff --git a/nginx/t/js_merge_location_blocks.t b/nginx/t/js_merge_location_blocks.t
new file mode 100644
index 00000000..328d5338
--- /dev/null
+++ b/nginx/t/js_merge_location_blocks.t
@@ -0,0 +1,83 @@
+#!/usr/bin/perl
+
+# (C) Dmitry Volyntsev
+# (c) Nginx, Inc.
+
+# Tests for http njs module, check for proper location blocks merging.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http/)
+ ->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ js_import main.js;
+
+ server {
+ listen 127.0.0.1:8080;
+ server_name localhost;
+
+ location /a {
+ js_content main.version;
+ }
+
+ location /b {
+ js_content main.version;
+ }
+
+ location /c {
+ js_content main.version;
+ }
+
+ location /d {
+ js_content main.version;
+ }
+ }
+}
+
+EOF
+
+$t->write_file('main.js', <<EOF);
+ function version(r) {
+ r.return(200, njs.version);
+ }
+
+ export default {version};
+
+EOF
+
+$t->try_run('no njs available')->plan(1);
+
+###############################################################################
+
+$t->stop();
+
+my $content = $t->read_file('error.log');
+my $count = () = $content =~ m/ js vm init/g;
+ok($count == 1, 'http js block imported once');
+
+###############################################################################
diff --git a/nginx/t/js_merge_server_blocks.t b/nginx/t/js_merge_server_blocks.t
new file mode 100644
index 00000000..c3f261af
--- /dev/null
+++ b/nginx/t/js_merge_server_blocks.t
@@ -0,0 +1,78 @@
+#!/usr/bin/perl
+
+# (C) Dmitry Volyntsev
+# (c) Nginx, Inc.
+
+# Tests for http njs module, check for proper server blocks merging.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http/)
+ ->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ js_import main.js;
+
+ server {
+ listen 127.0.0.1:8080;
+ }
+
+ server {
+ listen 127.0.0.1:8081;
+ }
+
+ server {
+ listen 127.0.0.1:8082;
+ }
+
+ server {
+ listen 127.0.0.1:8083;
+ }
+}
+
+EOF
+
+$t->write_file('main.js', <<EOF);
+ function version(r) {
+ r.return(200, njs.version);
+ }
+
+ export default {version};
+
+EOF
+
+$t->try_run('no njs available')->plan(1);
+
+###############################################################################
+
+$t->stop();
+
+my $content = $t->read_file('error.log');
+my $count = () = $content =~ m/ js vm init/g;
+ok($count == 1, 'http js block imported once');
+
+###############################################################################
diff --git a/nginx/t/stream_js.t b/nginx/t/stream_js.t
index 52ab688e..0834b68a 100644
--- a/nginx/t/stream_js.t
+++ b/nginx/t/stream_js.t
@@ -394,7 +394,7 @@ $t->write_file('test.js', <<EOF);
EOF

$t->run_daemon(\&stream_daemon, port(8090));
-$t->try_run('no stream njs available')->plan(24);
+$t->try_run('no stream njs available')->plan(25);
$t->waitforsocket('127.0.0.1:' . port(8090));

###############################################################################
@@ -450,6 +450,10 @@ like($t->read_file('status.log'), qr/$p[0]:200/, 'status undecided');
like($t->read_file('status.log'), qr/$p[1]:200/, 'status allow');
like($t->read_file('status.log'), qr/$p[2]:403/, 'status deny');

+my $content = $t->read_file('error.log');
+my $count = () = $content =~ m/ js vm init/g;
+ok($count == 2, 'http and stream js blocks imported once each');
+
###############################################################################

sub has_version {
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel