Re: [Tarantool-patches] [PATCH small 0/2] Aligned lsregion

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

On 15/05/2020 14:26, Aleksandr Lyapunov wrote:
> Thanks for the patch! see comments below.
> 
> On 5/15/20 2:31 AM, Vladislav Shpilevoy wrote:
>> The patchset introduces a new function - lsregion_aligned_alloc().
> 
> Actually you don't need the patch for fixing #4609.
> If you want to align vy_mem tuple by 4 bytes (due to uint32_t bsize), all you need is:
> 
> diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
> index adc3ba4..2de4a28 100644
> --- a/src/box/vy_stmt.c
> +++ b/src/box/vy_stmt.c
> @@ -228,7 +228,7 @@ vy_stmt_dup_lsregion(struct tuple *stmt, struct lsregion *lsregion,
>         if (type == IPROTO_UPSERT)
>                 alloc_size++;
>  
> -       mem_stmt = lsregion_alloc(lsregion, alloc_size, alloc_id);
> +       mem_stmt = lsregion_alloc(lsregion, small_align(alloc_size, 4), alloc_id);
>         if (mem_stmt == NULL) {
>                 diag_set(OutOfMemory, size, "lsregion_alloc", "mem_stmt");
>                 return NULL;
> 
> lsregion will allocate aligned addresses as long as you request only aligned sizes,
> up to sizeof(uintptr_t) (see lslab_sizeof()).
> This property is not documented (maybe it should be!). It is also similar to mempool
> property, see mempool_create description.

Yeah, here are 3 problems:
- lsregion is used not only for tuples, but for bps tree extents too. We can't
  rely on their alignment being the same. Sanitizer revealed, that both extents
  and tuples are not aligned;
- sanitizer revealed, that mempool_alloc returns unaligned addresses too. I am
  going to look into that after lsregion;
- slab first position can be aligned not enough. So we can't assume anything
  about first address in a slab. And therefore about all the other addresses
  too.

> I like one-line change more.
> 
> If you still want _reserve and _aligned_alloc method for further purposes, please
> read my by-commit comments.

I want lsregion be consistent with other allocators, and provide basic
API for aligned allocations. Like region does.