Re: [PATCH] jit_size: Don't round up the size to a multiple of 4 KiB

[Paulo César Pereira de Andrade]

Em dom., 5 de jun. de 2022 às 07:25, Paul Cercueil
<paul@crapouillou.net> escreveu:

  Hi,

  Applied, but some considerations below.

> When using an external code buffer, a program using Lightning will call
> jit_get_code() to get the expected size of the code, then allocate some
> space in the code buffer, then call jit_set_code() to specify where the
> code should be written to.

  There should be some api to reset _jit->user_code, and use the patch
based on if _jit->user_code. Also, likely instead of rounding to 4096 bytes,
round to pagesize. The rounding up is just to "adapt" to what mmap does.

> If the reported size is rounded up to a multiple of 4 KiB, then the
> allocator will always try to allocate 4 KiB for blocks of code that
> might even be smaller than a hundred bytes. The program can then choose
> to realloc() the allocated block to the actual size of the generated
> code, to reduce the memory used.

  It is required some extra logic to allow easy reutilization of a _jit context.
The current logic is good for significantly large jit buffers, but too costly
for code doing very small buffers. Resetting the internal state of a
_jit context
should be far faster than deleting a context and creating a new one.

> However, this will cause dramatic memory fragmentation; for instance,
> if working with a 2 MiB code buffer, in which 512 blocks of 4 KiB are
> allocated but later realloc'd to 128 bytes each, the total amount of
> allocated memory will be 128 * 512 == 64 KiB, with almost 1.9 MiB free,
> yet it will be impossible to allocate any new blocks as there would be
> no way to find a contiguous 4 KiB area.

  Resetting a jit_context could also have some "self healing" code, in case
it has a too large _jit->*.{count,length}. Most likely one to use more memory
is _jit->pool.ptr, if some very large jit code buffer was written (it
always rounds
up to 1024 free nodes when running out of nodes).

> Besides, I really don't understand why it was rounded up to a multiple
> of 4 KiB, as this is not a requirement for mmap().

  Usually the memory rounding up to 4096 will either not be accessible (and
not used by any other mmap call) or the code will just refuse to write to that
extra memory. It was done so, just in case it miscalculated by a few bytes
the code size, to not need to mmap or mremap again.  If mremap is available,
that logic is mostly pointless...

> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  lib/jit_size.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/jit_size.c b/lib/jit_size.c
> index 61f1aa4..3a78394 100644
> --- a/lib/jit_size.c
> +++ b/lib/jit_size.c
> @@ -105,7 +105,7 @@ _jit_get_size(jit_state_t *_jit)
>      for (size = JIT_INSTR_MAX, node = _jitc->head; node; node = node->next)
>         size += _szs[node->code];
>
> -    return ((size + 4095) & -4096);
> +    return size;
>  }
>  #endif
>
> --
> 2.35.1

Thanks!
Paulo