From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 85230469710 for ; Fri, 15 May 2020 15:26:17 +0300 (MSK) References: From: Aleksandr Lyapunov Message-ID: Date: Fri, 15 May 2020 15:26:15 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/alternative; boundary="------------92E41C62CA7DAE645D215F2D" Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH small 0/2] Aligned lsregion List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org, korablev@tarantool.org, tsafin@tarantool.org, gorcunov@gmail.com This is a multi-part message in MIME format. --------------92E41C62CA7DAE645D215F2D Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit 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. I like one-line change more. If you still want _reserve and _aligned_alloc method for further purposes, please read my by-commit comments. --------------92E41C62CA7DAE645D215F2D Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

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.

I like one-line change more.

If you still want _reserve and _aligned_alloc method for further purposes, please
read my by-commit comments.

--------------92E41C62CA7DAE645D215F2D--