Skip to content

Simple fixes to reduce memory consumption. #935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Simple fixes to reduce memory consumption. #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cef52d8
NJS_VMCODE_ADDITION: strings for numbers allocated on stack.
VadimZhestikov Jun 23, 2025
67a5f19
String concat: strings for numbers allocated on stack.
VadimZhestikov Jun 24, 2025
39d008d
Released temporary template literal memory.
VadimZhestikov Jun 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions src/njs_string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,19 +601,56 @@ njs_int_t
njs_string_prototype_concat(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
njs_index_t unused, njs_value_t *retval)
{
double num;
u_char *p, *start;
uint64_t size, length, mask;
njs_int_t ret;
njs_uint_t i;
njs_uint_t i, i_buf, n_bufs;
njs_string_prop_t string;

if (njs_is_null_or_undefined(&args[0])) {
njs_type_error(vm, "\"this\" argument is null or undefined");
return NJS_ERROR;
}

n_bufs = njs_min(nargs,16);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to improve it further by using njs chain buffers. It allows us to write numbers directly to a future output buffer without intermediary values.

diff --git a/src/njs_string.c b/src/njs_string.c
index cf3e1393..c0689838 100644
--- a/src/njs_string.c
+++ b/src/njs_string.c
@@ -602,10 +602,11 @@ njs_string_prototype_concat(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     njs_index_t unused, njs_value_t *retval)
 {
     double             num;
-    u_char             *p, *start;
-    uint64_t           size, length, mask;
+    u_char             *dst, *start;
+    uint64_t           size, length;
     njs_int_t          ret;
-    njs_uint_t         i, i_buf, n_bufs;
+    njs_chb_t          chain;
+    njs_uint_t         i;
     njs_string_prop_t  string;
 
     if (njs_is_null_or_undefined(&args[0])) {
@@ -613,82 +614,76 @@ njs_string_prototype_concat(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         return NJS_ERROR;
     }
 
-    n_bufs = njs_min(nargs,16);
+    if (nargs == 1) {
+        njs_value_assign(retval, &args[0]);
+        return NJS_OK;
+    }
 
-    u_char        buf[n_bufs][32];
-    njs_string_t  sp[n_bufs];
+    length = 0;
 
-    i_buf = 0;
+    NJS_CHB_MP_INIT(&chain, njs_vm_memory_pool(vm));
 
     for (i = 0; i < nargs; i++) {
-        if (i_buf < n_bufs && njs_is_number(&args[i])) {
+        if (njs_is_number(&args[i])) {
             num = njs_number(&args[i]);
 
             if (isnan(num)) {
-                njs_atom_to_value(vm, &args[i], NJS_ATOM_STRING_NaN);
+                njs_chb_append(&chain, "NaN", 3);
+                length += 3;
 
             } else if (isinf(num)) {
 
                 if (num < 0) {
-                    njs_atom_to_value(vm, &args[i], NJS_ATOM_STRING__Infinity);
+                    njs_chb_append(&chain, "-Infinity", 9);
+                    length += 9;
 
                 } else {
-                    njs_atom_to_value(vm, &args[i], NJS_ATOM_STRING_Infinity);
+                    njs_chb_append(&chain, "Infinity", 8);
+                    length += 8;
                 }
 
             } else {
-                size = njs_dtoa(num, (char *) buf[i]);
+                dst = njs_chb_reserve(&chain, 64);
+                if (njs_slow_path(dst == NULL)) {
+                    return NJS_ERROR;
+                }
 
-                sp[i_buf].start = (u_char *)buf[i];
-                sp[i_buf].length = sp[i_buf].size = size;
+                size = njs_dtoa(num, (char *) dst);
+                njs_chb_written(&chain, size);
+                length += size;
+            }
 
-                args[i].type = NJS_STRING;
-                args[i].truth = size != 0;
-                args[i].atom_id = NJS_ATOM_STRING_unknown;
-                args[i].string.data = &sp[i_buf];
+        } else if (njs_is_string(&args[i])) {
+string:
+            njs_string_prop(vm, &string, &args[i]);
 
-                i_buf++;
-            }
+            njs_chb_append(&chain, string.start, string.size);
+            length += string.length;
 
-        } if (!njs_is_string(&args[i])) {
+        } else {
             ret = njs_value_to_string(vm, &args[i], &args[i]);
             if (ret != NJS_OK) {
                 return ret;
             }
-        }
-    }
 
-    if (nargs == 1) {
-        njs_string_copy(retval, &args[0]);
-        return NJS_OK;
+            goto string;
+        }
     }
 
-    size = 0;
-    length = 0;
-    mask = -1;
-
-    for (i = 0; i < nargs; i++) {
-        (void) njs_string_prop(vm, &string, &args[i]);
-
-        size += string.size;
-        length += string.length;
+    size = njs_chb_size(&chain);
+    if (njs_slow_path(size < 0)) {
+        njs_memory_error(vm);
+        return NJS_ERROR;
     }
 
-    length &= mask;
-
     start = njs_string_alloc(vm, retval, size, length);
     if (njs_slow_path(start == NULL)) {
         return NJS_ERROR;
     }
 
-    p = start;
-
-    for (i = 0; i < nargs; i++) {
-        (void) njs_string_prop(vm, &string, &args[i]);
+    njs_chb_join_to(&chain, start);
 
-        p = memcpy(p, string.start, string.size);
-        p += string.size;
-    }
+    njs_chb_destroy(&chain);
 
     return NJS_OK;
 }


u_char buf[n_bufs][32];
njs_string_t sp[n_bufs];

i_buf = 0;

for (i = 0; i < nargs; i++) {
if (!njs_is_string(&args[i])) {
if (i_buf < n_bufs && njs_is_number(&args[i])) {
num = njs_number(&args[i]);

if (isnan(num)) {
njs_atom_to_value(vm, &args[i], NJS_ATOM_STRING_NaN);

} else if (isinf(num)) {

if (num < 0) {
njs_atom_to_value(vm, &args[i], NJS_ATOM_STRING__Infinity);

} else {
njs_atom_to_value(vm, &args[i], NJS_ATOM_STRING_Infinity);
}

} else {
size = njs_dtoa(num, (char *) buf[i]);

sp[i_buf].start = (u_char *)buf[i];
sp[i_buf].length = sp[i_buf].size = size;

args[i].type = NJS_STRING;
args[i].truth = size != 0;
args[i].atom_id = NJS_ATOM_STRING_unknown;
args[i].string.data = &sp[i_buf];

i_buf++;
}

} if (!njs_is_string(&args[i])) {
ret = njs_value_to_string(vm, &args[i], &args[i]);
if (ret != NJS_OK) {
return ret;
Expand Down
54 changes: 47 additions & 7 deletions src/njs_vmcode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,14 +667,52 @@ NEXT_LBL;
src = value1;
}

ret = njs_primitive_value_to_string(vm, &dst, src);
if (njs_slow_path(ret != NJS_OK)) {
goto error;
}
if (src->type == NJS_NUMBER) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not observe memory or performance improvements for both bench4 or the provided test.
If this is the case, I suggest to drop this patch.


ret = njs_string_concat(vm, s1, s2, &name);
if (njs_slow_path(ret == NJS_ERROR)) {
goto error;
double num;
size_t size;
u_char buf[32];

njs_string_t sp;

num = njs_number(src);

if (isnan(num)) {
njs_atom_to_value(vm, &dst, NJS_ATOM_STRING_NaN);

} else if (isinf(num)) {

if (num < 0) {
njs_atom_to_value(vm, &dst, NJS_ATOM_STRING__Infinity);

} else {
njs_atom_to_value(vm, &dst, NJS_ATOM_STRING_Infinity);
}

} else {
size = njs_dtoa(num, (char *) buf);

sp.start = (u_char *)buf;
sp.length = sp.size = size;

dst.string.data = &sp;
}

ret = njs_string_concat(vm, s1, s2, &name);
if (njs_slow_path(ret == NJS_ERROR)) {
goto error;
}

} else {
ret = njs_primitive_value_to_string(vm, &dst, src);
if (njs_slow_path(ret != NJS_OK)) {
goto error;
}

ret = njs_string_concat(vm, s1, s2, &name);
if (njs_slow_path(ret == NJS_ERROR)) {
goto error;
}
}

njs_value_assign(retval, &name);
Expand Down Expand Up @@ -2010,6 +2048,8 @@ njs_vmcode_template_literal(njs_vm_t *vm, njs_value_t *retval)
return ret;
}

njs_array_destroy(vm, array);

return sizeof(njs_vmcode_template_literal_t);
}

Expand Down