Skip to content

Use calloc rather than malloc + memset #519

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

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Use calloc rather than malloc + memset #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f87036
Use calloc rather than malloc + memset
saghul Sep 10, 2024
13d21f2
Simplify realloc
saghul Sep 10, 2024
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
36 changes: 30 additions & 6 deletions qjs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,29 @@ static void js_trace_malloc_init(struct trace_malloc_data *s)
free(s->base = malloc(8));
}

static void *js_trace_calloc(JSMallocState *s, size_t count, size_t size)
{
void *ptr;

/* Do not allocate zero bytes: behavior is platform dependent */
assert(count != 0 && size != 0);

if (size > 0)
if (unlikely(count != (count * size) / size))
return NULL;

/* When malloc_limit is 0 (unlimited), malloc_limit - 1 will be SIZE_MAX. */
if (unlikely(s->malloc_size + (count * size) > s->malloc_limit - 1))
Copy link
Contributor

Choose a reason for hiding this comment

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

count * size can overflow, you should probably handle that:

if (size > 0) // I know you have an assert a few lines up but just in case
    if (count != (count * size) / size)
        return NULL;

Copy link
Contributor

Choose a reason for hiding this comment

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

An interesting tidbit... I was going to suggest to do the overflow check like this:

if (size > 0)
    if (count > SIZE_MAX / size)
        return NULL;

...on the assumption it's faster because it's replacing a variable (count) with a constant (SIZE_MAX) but... it's not!

gcc at -O3 compiles it to mul + jo (jump if overflow) in both cases, but in the SIZE_MAX code, it prepends a test + jz (jump if zero). Must be a missed optimization because omitting the test + jz doesn't affect correctness.

Even more interestingly: gcc replaces malloc+memset with calloc calls!

Code:

void *alloc(size_t m, size_t n) {
#ifdef DIVIDE_CONSTANT
        if (n > 0 && m > (size_t)-1 / n) return 0;
#else
        if (n > 0 && m != (m * n) / n) return 0;
#endif
        void *p = malloc(m*n);
        if (p) memset(p, 0, m*n);
        return p;
}

Assembly:

// -DDIVIDE_CONSTANT
0000000000001170 <alloc>:
    1170:       f3 0f 1e fa             endbr64 
    1174:       48 85 f6                test   %rsi,%rsi
    1177:       74 08                   je     1181 <alloc+0x11>
    1179:       48 89 f8                mov    %rdi,%rax
    117c:       48 f7 e6                mul    %rsi
    117f:       70 0e                   jo     118f <alloc+0x1f>
    1181:       48 0f af fe             imul   %rsi,%rdi
    1185:       be 01 00 00 00          mov    $0x1,%esi
    118a:       e9 c1 fe ff ff          jmp    1050 <calloc@plt>
    118f:       31 c0                   xor    %eax,%eax
    1191:       c3                      ret    

// no -DDIVIDE_CONSTANT
0000000000001170 <alloc>:
    1170:       f3 0f 1e fa             endbr64 
    1174:       48 89 f0                mov    %rsi,%rax
    1177:       48 f7 e7                mul    %rdi
    117a:       48 89 c7                mov    %rax,%rdi
    117d:       70 0a                   jo     1189 <alloc+0x19>
    117f:       be 01 00 00 00          mov    $0x1,%esi
    1184:       e9 c7 fe ff ff          jmp    1050 <calloc@plt>
    1189:       31 c0                   xor    %eax,%eax
    118b:       c3                      ret

return NULL;
ptr = calloc(count, size);
js_trace_malloc_printf(s, "C %zd %zd -> %p\n", count, size, ptr);
if (ptr) {
s->malloc_count++;
s->malloc_size += js__malloc_usable_size(ptr) + MALLOC_OVERHEAD;
}
return ptr;
}

static void *js_trace_malloc(JSMallocState *s, size_t size)
{
void *ptr;
Expand Down Expand Up @@ -265,14 +288,14 @@ static void *js_trace_realloc(JSMallocState *s, void *ptr, size_t size)
return NULL;
return js_trace_malloc(s, size);
}
old_size = js__malloc_usable_size(ptr);
if (size == 0) {
js_trace_malloc_printf(s, "R %zd %p\n", size, ptr);
s->malloc_count--;
s->malloc_size -= old_size + MALLOC_OVERHEAD;
free(ptr);

if (unlikely(size == 0)) {
js_trace_free(s, ptr);
return NULL;
}

old_size = js__malloc_usable_size(ptr);

/* When malloc_limit is 0 (unlimited), malloc_limit - 1 will be SIZE_MAX. */
if (s->malloc_size + size - old_size > s->malloc_limit - 1)
return NULL;
Expand All @@ -288,6 +311,7 @@ static void *js_trace_realloc(JSMallocState *s, void *ptr, size_t size)
}

static const JSMallocFunctions trace_mf = {
js_trace_calloc,
js_trace_malloc,
js_trace_free,
js_trace_realloc,
Expand Down
66 changes: 54 additions & 12 deletions quickjs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,11 @@ static size_t js_malloc_usable_size_unknown(const void *ptr)
return 0;
}

void *js_calloc_rt(JSRuntime *rt, size_t count, size_t size)
{
return rt->mf.js_calloc(&rt->malloc_state, count, size);
}

void *js_malloc_rt(JSRuntime *rt, size_t size)
{
return rt->mf.js_malloc(&rt->malloc_state, size);
Expand All @@ -1404,13 +1409,16 @@ size_t js_malloc_usable_size_rt(JSRuntime *rt, const void *ptr)
return rt->mf.js_malloc_usable_size(ptr);
}

/**
* This used to be implemented as malloc + memset, but using calloc
* yields better performance in initial, bursty allocations, something useful
* for QuickJS.
*
* More information: https://github.com/quickjs-ng/quickjs/pull/519
*/
void *js_mallocz_rt(JSRuntime *rt, size_t size)
{
void *ptr;
ptr = js_malloc_rt(rt, size);
if (!ptr)
return NULL;
return memset(ptr, 0, size);
return js_calloc_rt(rt, 1, size);
}

/* called by libbf */
Expand All @@ -1420,6 +1428,18 @@ static void *js_bf_realloc(void *opaque, void *ptr, size_t size)
return js_realloc_rt(rt, ptr, size);
}

/* Throw out of memory in case of error */
void *js_calloc(JSContext *ctx, size_t count, size_t size)
{
void *ptr;
ptr = js_calloc_rt(ctx->rt, count, size);
if (unlikely(!ptr)) {
JS_ThrowOutOfMemory(ctx);
return NULL;
}
return ptr;
}

/* Throw out of memory in case of error */
void *js_malloc(JSContext *ctx, size_t size)
{
Expand Down Expand Up @@ -1631,10 +1651,9 @@ JSRuntime *JS_NewRuntime2(const JSMallocFunctions *mf, void *opaque)
ms.opaque = opaque;
ms.malloc_limit = 0;

rt = mf->js_malloc(&ms, sizeof(JSRuntime));
rt = mf->js_calloc(&ms, 1, sizeof(JSRuntime));
if (!rt)
return NULL;
memset(rt, 0, sizeof(*rt));
rt->mf = *mf;
if (!rt->mf.js_malloc_usable_size) {
/* use dummy function if none provided */
Expand Down Expand Up @@ -1699,6 +1718,30 @@ void JS_SetRuntimeOpaque(JSRuntime *rt, void *opaque)
rt->user_opaque = opaque;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

}

static void *js_def_calloc(JSMallocState *s, size_t count, size_t size)
{
void *ptr;

/* Do not allocate zero bytes: behavior is platform dependent */
assert(count != 0 && size != 0);

if (size > 0)
if (unlikely(count != (count * size) / size))
return NULL;

/* When malloc_limit is 0 (unlimited), malloc_limit - 1 will be SIZE_MAX. */
if (unlikely(s->malloc_size + (count * size) > s->malloc_limit - 1))
return NULL;

ptr = calloc(count, size);
if (!ptr)
return NULL;

s->malloc_count++;
s->malloc_size += js__malloc_usable_size(ptr) + MALLOC_OVERHEAD;
return ptr;
}

static void *js_def_malloc(JSMallocState *s, size_t size)
{
void *ptr;
Expand Down Expand Up @@ -1738,13 +1781,11 @@ static void *js_def_realloc(JSMallocState *s, void *ptr, size_t size)
return NULL;
return js_def_malloc(s, size);
}
old_size = js__malloc_usable_size(ptr);
if (size == 0) {
s->malloc_count--;
s->malloc_size -= old_size + MALLOC_OVERHEAD;
free(ptr);
if (unlikely(size == 0)) {
js_def_free(s, ptr);
return NULL;
}
old_size = js__malloc_usable_size(ptr);
/* When malloc_limit is 0 (unlimited), malloc_limit - 1 will be SIZE_MAX. */
if (s->malloc_size + size - old_size > s->malloc_limit - 1)
return NULL;
Expand All @@ -1758,6 +1799,7 @@ static void *js_def_realloc(JSMallocState *s, void *ptr, size_t size)
}

static const JSMallocFunctions def_malloc_funcs = {
js_def_calloc,
js_def_malloc,
js_def_free,
js_def_realloc,
Expand Down
3 changes: 3 additions & 0 deletions quickjs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ typedef struct JSMallocState {
} JSMallocState;

typedef struct JSMallocFunctions {
void *(*js_calloc)(JSMallocState *s, size_t count, size_t size);
void *(*js_malloc)(JSMallocState *s, size_t size);
void (*js_free)(JSMallocState *s, void *ptr);
void *(*js_realloc)(JSMallocState *s, void *ptr, size_t size);
Expand Down Expand Up @@ -358,12 +359,14 @@ JS_EXTERN JS_BOOL JS_IsSameValueZero(JSContext *ctx, JSValue op1, JSValue op2);
JS_EXTERN JSValue js_string_codePointRange(JSContext *ctx, JSValue this_val,
int argc, JSValue *argv);

JS_EXTERN void *js_calloc_rt(JSRuntime *rt, size_t count, size_t size);
JS_EXTERN void *js_malloc_rt(JSRuntime *rt, size_t size);
JS_EXTERN void js_free_rt(JSRuntime *rt, void *ptr);
JS_EXTERN void *js_realloc_rt(JSRuntime *rt, void *ptr, size_t size);
JS_EXTERN size_t js_malloc_usable_size_rt(JSRuntime *rt, const void *ptr);
JS_EXTERN void *js_mallocz_rt(JSRuntime *rt, size_t size);

JS_EXTERN void *js_calloc(JSContext *ctx, size_t count, size_t size);
JS_EXTERN void *js_malloc(JSContext *ctx, size_t size);
JS_EXTERN void js_free(JSContext *ctx, void *ptr);
JS_EXTERN void *js_realloc(JSContext *ctx, void *ptr, size_t size);
Expand Down
Loading