MPR #6820

Any comment on the proposal? Is Obj the appropriate module to put this function? I'd also appreciate if another pair of eyes could review the code; it's easy to make a mistake...

Is Obj the appropriate module to put this function?

I think it's better than the Gc module proposed in MPR #6820.

I only had a very quick look (and am jetlagged) but it looks like caml_stat_alloc should probably be used instead of malloc, no?

I'm a bit torn about Gc versus Obj. In some sense, if this is going to be a safe operation, there's no need for it to be in Obj. However it doesn't seem to quite fit into Gc either.

I only had a very quick look (and am jetlagged) but it looks like caml_stat_alloc should probably be used instead of malloc, no?

I got inspired by extern.c which uses malloc. A quick grep-fight in byterun gives an inconclusive:

$ grep malloc *.c  | wc -l
34

$ grep caml_stat_alloc *.c  | wc -l
32

Does anyone know how to choose?

In some sense, if this is going to be a safe operation,

Obj.size is also safe (I think). But such function are very implementation dependent. Typically, it's not clear than javascript backend will support reachable_words (with a close-enough semantics -- I'm thinking about their ability to look into closures). Perhaps Sys?

I think Obj is better than Sys.

@damiendoligez Can you comment about malloc vs caml_stat_alloc?

I am really surprised by this merge. I was thinking that we were waiting for Damien's feedback, and that the technical discussion was not over, as I think the primitive you described is far from being as useful as you said, in its current state (at least, for a cache or whatever, I would expect the primitive to come with a threshold argument, that would stop the traversal if the size becomes greater than the threshold).
@alainfrisch @damiendoligez

Damien posted some (minor) comments, which I addressed. I guess that he would have said so if he was against the proposal.

The threshold is indeed a good idea for use in caches, but the current version is already very useful for debugging purposes.

@alainfrisch I think that one big difference is that if the runtime becomes unloadable, then caml_stat_alloc will use a memory pool to ensure all allocations are freed.

CI fails with flambda.

@drbo This means we should be using caml_stat_alloc everywhere instead of malloc, right?

@alainfrisch minor comments don't really mean that I approve of everything else, especially when I don't add the "approved" label...

@damiendoligez I would imagine so (not part of the core team).

FWIW, we (the Frama-C team) have been using @backtracking's size library for a long time. This is more or less what is proposed here. Our experience is that more functions are needed very rapidly. In particular, we often need to compute "the size of this value except what is also reachable from that value".

In particular, we often need to compute "the size of this value except what is also reachable from that value".

This would be quite easy to implement, but costly if "that" value itself is big. A variant is to "cut" only one (or several) specific values (but not blocks accessible from them).

This means we should be using caml_stat_alloc everywhere instead of malloc, right?

Not always. Only if you're happy with caml_stat_alloc aborting the current function if it runs out of memory. However, this can lead to memory leaks. Consider two out-of-heap allocations in sequence:

  char * buf1 = caml_stat_alloc(size1);
  char * buf2 = caml_stat_alloc(size2);

If the second allocation runs out of memory (but not the first), an exception is raised and the space for buf1 will never be released. In such cases, it is better to write

  char * buf1 = malloc(size1);
  if (buf1 == NULL) caml_raise_out_of_memory();
  char * buf2 = malloc(size2);
  if (buf2 == NULL) { free(buf1); caml_raise_out_of_memory(); }

OK, so don't use caml_stat_alloc as a replacement for malloc. We'll have to review all uses of malloc to implement the unloadable runtime anyway.