Skip to content

Commit

Permalink
Ensure that all Bigarray memory managed by the GC is tracked by Mempr…
Browse files Browse the repository at this point in the history
…of (#3356)

There were two cases where this was not already the case:

  - Unix.map_file:
    Fixed by adding memprof calls to mmap_ba.c

  - User-allocated, GC-freed bigstrings:
    These are allocated by calls to caml_ba_alloc with CAML_BA_MANAGED,
    non-NULL data, and not CAML_BA_SUBARRAY (so new bigarrays, not slices)

    Runtime5 already handled this case correctly. The fix for runtime4
    is to add a new call to memprof for the missing case.
  • Loading branch information
stedolan authored Dec 11, 2024
1 parent 26794a5 commit 8d31575
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 24 deletions.
15 changes: 13 additions & 2 deletions otherlibs/unix/mmap_ba.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "caml/custom.h"
#include "caml/memory.h"
#include "caml/misc.h"
#include "caml/memprof.h"

/* Allocation of bigarrays for memory-mapped files.
This is the OS-independent part of [mmap.c]. */
Expand Down Expand Up @@ -60,17 +61,27 @@ static struct custom_operations caml_ba_mapped_ops = {
CAMLexport value
caml_unix_mapped_alloc(int flags, int num_dims, void * data, intnat * dim)
{
uintnat asize;
uintnat asize, num_elts = 1, mem_bytes, mem_words;
int i;
value res;
struct caml_ba_array * b;
intnat dimcopy[CAML_BA_MAX_NUM_DIMS];

CAMLassert(num_dims >= 0 && num_dims <= CAML_BA_MAX_NUM_DIMS);
CAMLassert((flags & CAML_BA_KIND_MASK) < CAML_BA_FIRST_UNIMPLEMENTED_KIND);
for (i = 0; i < num_dims; i++) dimcopy[i] = dim[i];
for (i = 0; i < num_dims; i++) {
num_elts *= dim[i];
dimcopy[i] = dim[i];
}
asize = SIZEOF_BA_ARRAY + num_dims * sizeof(intnat);
res = caml_alloc_custom(&caml_ba_mapped_ops, asize, 0, 1);
mem_bytes = num_elts * caml_ba_element_size[flags & CAML_BA_KIND_MASK];
mem_words = (mem_bytes + sizeof(value) - 1) / sizeof(value);
#ifdef CAML_RUNTIME_5
caml_memprof_sample_block(res, mem_words, mem_words, CAML_MEMPROF_SRC_CUSTOM);
#else
caml_memprof_track_custom(res, mem_bytes);
#endif
b = Caml_ba_array_val(res);
b->data = data;
b->num_dims = num_dims;
Expand Down
52 changes: 33 additions & 19 deletions runtime4/bigarray.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "caml/memory.h"
#include "caml/mlvalues.h"
#include "caml/signals.h"
#include "caml/memprof.h"

#define int8 caml_ba_int8
#define uint8 caml_ba_uint8
Expand Down Expand Up @@ -223,32 +224,41 @@ CAMLexport struct custom_operations caml_ba_ops = {
CAMLexport value
caml_ba_alloc(int flags, int num_dims, void * data, intnat * dim)
{
uintnat num_elts, asize, size;
int i;
uintnat num_elts, asize, mem_size, alloc_size;
int i, extra_track;
value res;
struct caml_ba_array * b;
intnat dimcopy[CAML_BA_MAX_NUM_DIMS];

CAMLassert(num_dims >= 0 && num_dims <= CAML_BA_MAX_NUM_DIMS);
CAMLassert((flags & CAML_BA_KIND_MASK) < CAML_BA_FIRST_UNIMPLEMENTED_KIND);
for (i = 0; i < num_dims; i++) dimcopy[i] = dim[i];
size = 0;
if (data == NULL) {
num_elts = 1;
for (i = 0; i < num_dims; i++) {
if (caml_umul_overflow(num_elts, dimcopy[i], &num_elts))
caml_raise_out_of_memory();
}
if (caml_umul_overflow(num_elts,
caml_ba_element_size[flags & CAML_BA_KIND_MASK],
&size))
num_elts = 1;
for (i = 0; i < num_dims; i++) {
if (caml_umul_overflow(num_elts, dimcopy[i], &num_elts))
caml_raise_out_of_memory();
data = malloc(size);
if (data == NULL && size != 0) caml_raise_out_of_memory();
}
if (caml_umul_overflow(num_elts,
caml_ba_element_size[flags & CAML_BA_KIND_MASK],
&mem_size))
caml_raise_out_of_memory();
alloc_size = 0;
extra_track =
/* Backport runtime5's memprof logic: Under the below conditions,
we want a memprof entry (it's GC-managed memory), but runtime4's
call to caml_alloc_custom_mem will not create one. */
data != NULL &&
((flags & CAML_BA_MANAGED_MASK) == CAML_BA_MANAGED) &&
!(flags & CAML_BA_SUBARRAY);
if (data == NULL) {
alloc_size = mem_size;
data = malloc(alloc_size);
if (data == NULL && alloc_size != 0) caml_raise_out_of_memory();
flags |= CAML_BA_MANAGED;
}
asize = SIZEOF_BA_ARRAY + num_dims * sizeof(intnat);
res = caml_alloc_custom_mem(&caml_ba_ops, asize, size);
res = caml_alloc_custom_mem(&caml_ba_ops, asize, alloc_size);
if (extra_track) caml_memprof_track_custom(res, mem_size);
b = Caml_ba_array_val(res);
b->data = data;
b->num_dims = num_dims;
Expand Down Expand Up @@ -1176,7 +1186,8 @@ CAMLprim value caml_ba_slice(value vb, value vind)
(char *) b->data +
offset * caml_ba_element_size[b->flags & CAML_BA_KIND_MASK];
/* Allocate an OCaml bigarray to hold the result */
res = caml_ba_alloc(b->flags, b->num_dims - num_inds, sub_data, sub_dims);
res = caml_ba_alloc(b->flags | CAML_BA_SUBARRAY,
b->num_dims - num_inds, sub_data, sub_dims);
/* Copy the finalization function from the original array (PR#8568) */
Custom_ops_val(res) = Custom_ops_val(vb);
/* Create or update proxy in case of managed bigarray */
Expand All @@ -1203,7 +1214,8 @@ CAMLprim value caml_ba_change_layout(value vb, value vlayout)
intnat new_dim[CAML_BA_MAX_NUM_DIMS];
unsigned int i;
for(i = 0; i < b->num_dims; i++) new_dim[i] = b->dim[b->num_dims - i - 1];
res = caml_ba_alloc(flags, b->num_dims, b->data, new_dim);
res = caml_ba_alloc(flags | CAML_BA_SUBARRAY,
b->num_dims, b->data, new_dim);
/* Copy the finalization function from the original array (PR#8568) */
Custom_ops_val(res) = Custom_ops_val(vb);
caml_ba_update_proxy(b, Caml_ba_array_val(res));
Expand Down Expand Up @@ -1248,7 +1260,8 @@ CAMLprim value caml_ba_sub(value vb, value vofs, value vlen)
(char *) b->data +
ofs * mul * caml_ba_element_size[b->flags & CAML_BA_KIND_MASK];
/* Allocate an OCaml bigarray to hold the result */
res = caml_ba_alloc(b->flags, b->num_dims, sub_data, b->dim);
res = caml_ba_alloc(b->flags | CAML_BA_SUBARRAY,
b->num_dims, sub_data, b->dim);
/* Copy the finalization function from the original array (PR#8568) */
Custom_ops_val(res) = Custom_ops_val(vb);
/* Doctor the changed dimension */
Expand Down Expand Up @@ -1431,7 +1444,8 @@ CAMLprim value caml_ba_reshape(value vb, value vdim)
if (num_elts != caml_ba_num_elts(b))
caml_invalid_argument("Bigarray.reshape: size mismatch");
/* Create bigarray with same data and new dimensions */
res = caml_ba_alloc(b->flags, num_dims, b->data, dim);
res = caml_ba_alloc(b->flags | CAML_BA_SUBARRAY,
num_dims, b->data, dim);
/* Copy the finalization function from the original array (PR#8568) */
Custom_ops_val(res) = Custom_ops_val(vb);
/* Create or update proxy in case of managed bigarray */
Expand Down
5 changes: 2 additions & 3 deletions runtime4/caml/bigarray.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ enum caml_ba_managed {
};

enum caml_ba_subarray {
CAML_BA_SUBARRAY = 0 /* Data is shared with another bigarray
(Has no effect on runtime4, but present
for compatibility with runtime5) */
CAML_BA_SUBARRAY = 0x800 /* Data is shared with another bigarray
(runtime4: only affects Gc.Memprof events) */
};

struct caml_ba_proxy {
Expand Down
90 changes: 90 additions & 0 deletions testsuite/tests/statmemprof/bigarray.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
(* TEST
modules = "bigarray_stubs.c";
include unix;
hasunix;
{
bytecode;
}{
native;
}
*)
module MP = Gc.Memprof

type bigstring = (char, Bigarray.int8_unsigned_elt, Bigarray.c_layout) Bigarray.Array1.t

external static_bigstring : unit -> bigstring = "static_bigstring"
external new_bigstring : unit -> bigstring = "new_bigstring"
external malloc_bigstring : unit -> bigstring = "malloc_bigstring"

let bigstring_create sz : bigstring =
Bigarray.Array1.create Bigarray.char Bigarray.c_layout sz

let keep = ref []

let test () =
let custom_words = ref 0 in
let tmp_filename = Filename.temp_file "custom_test" ".dat" in
let alloc (info : MP.allocation) =
match info.source with
| Custom ->
custom_words := !custom_words + info.size * Sys.word_size/8;
Some info.size
| _ ->
None
in
let dealloc size =
custom_words := !custom_words - size * Sys.word_size/8
in
let tracker : _ MP.tracker =
{ alloc_minor = alloc;
alloc_major = alloc;
promote = (fun x -> Some x);
dealloc_minor = dealloc;
dealloc_major = dealloc }
in
let _:MP.t = MP.start ~sampling_rate:1. tracker in
let log s = Printf.printf "%20s: %d bytes\n%!" s !custom_words in
let[@inline never] test_tail () =
(* This is a separate tail-called function, to ensure
that [str] is out of scope even on bytecode builds *)
keep := [];
Gc.full_major ();
log "gc"
in
let test msg str =
Sys.poll_actions ();
log msg;
keep := [str];
(* sub and slice should not count as allocations *)
keep := Bigarray.Array1.sub str 1000 1000 :: !keep;
log "sub";
Gc.full_major ();
keep := Bigarray.Array1.sub str 1000 1000 :: !keep;
log "slice";
(test_tail[@tailcall]) ()
in
test "Allocation" (bigstring_create 5000);

let map_len = 64 * 1024 in
Unix.truncate tmp_filename map_len;
let fd = Unix.openfile tmp_filename [O_RDONLY] 0o600 in
test "Unix.map_file"
(Unix.map_file fd Bigarray.char Bigarray.c_layout false [| map_len |]
|> Bigarray.array1_of_genarray);
Unix.close fd;

(* Externally managed memory, should not be tracked *)
test "CAML_BA_EXTERNAL" (static_bigstring ());

(* Runtime-allocated memory, should be tracked *)
test "ba_alloc NULL" (new_bigstring ());

(* User-allocated yet GC-managed memory, should be tracked *)
test "CAML_BA_MANAGED" (malloc_bigstring ());

MP.stop ();
Sys.remove tmp_filename;
assert (!custom_words = 0)


let () = test ()
20 changes: 20 additions & 0 deletions testsuite/tests/statmemprof/bigarray.reference
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
Allocation: 5000 bytes
sub: 5000 bytes
slice: 5000 bytes
gc: 0 bytes
Unix.map_file: 65536 bytes
sub: 65536 bytes
slice: 65536 bytes
gc: 0 bytes
CAML_BA_EXTERNAL: 0 bytes
sub: 0 bytes
slice: 0 bytes
gc: 0 bytes
ba_alloc NULL: 5000 bytes
sub: 5000 bytes
slice: 5000 bytes
gc: 0 bytes
CAML_BA_MANAGED: 5000 bytes
sub: 5000 bytes
slice: 5000 bytes
gc: 0 bytes
24 changes: 24 additions & 0 deletions testsuite/tests/statmemprof/bigarray_stubs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#include <stdlib.h>
#include <caml/bigarray.h>

static char buf[10000];
value static_bigstring(value unit)
{
intnat dim[] = { sizeof(buf) };
return caml_ba_alloc(CAML_BA_UINT8 | CAML_BA_C_LAYOUT | CAML_BA_EXTERNAL,
1, buf, dim);
}

value new_bigstring(value unit)
{
intnat dim[] = { 5000 };
return caml_ba_alloc(CAML_BA_UINT8 | CAML_BA_C_LAYOUT,
1, NULL, dim);
}

value malloc_bigstring(value unit)
{
intnat dim[] = { 5000 };
return caml_ba_alloc(CAML_BA_UINT8 | CAML_BA_C_LAYOUT | CAML_BA_MANAGED,
1, malloc(dim[0]), dim);
}

0 comments on commit 8d31575

Please sign in to comment.