From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f194.google.com (mail-lj1-f194.google.com [209.85.208.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 39D99469710 for ; Mon, 8 Jun 2020 17:02:29 +0300 (MSK) Received: by mail-lj1-f194.google.com with SMTP id y11so18977275ljm.9 for ; Mon, 08 Jun 2020 07:02:29 -0700 (PDT) Date: Mon, 8 Jun 2020 17:02:26 +0300 From: Cyrill Gorcunov Message-ID: <20200608140226.GH134822@grain> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 07/10] vinyl: align statements and bps tree extents List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org On Thu, May 21, 2020 at 10:37:30PM +0200, Vladislav Shpilevoy wrote: > Vinyl tuples (vy_stmt) in 0 level of LSM tree are stored in > lsregion. They were allocated using lsregion_alloc(), which does > not align its results, and is good only for byte arrays. > > As a result, vy_stmt object addresses in 0 LSM level were not > aligned. Unaligned memory access is slower, and may even crash on > some platforms. > > Besides, even aligned allocations couldn't help upserts in 0 level > of the LSM tree, because upsert vy_stmt objects had 1 byte prefix > to count merged upserts stored in this statement. This 1 byte > prefix ruined all the alignment. Now the upsert counter is also > aligned, the same as vy_stmt. Note, it does not consume > significantly more memory, since it used only for vinyl and only > for upserts, stored in 0 level of the LSM tree. > > The same about BPS tree extents. LSM 0 level is a BPS tree, whose > blocks are allocated on lsregion. The extents are used as pointer > arrays inside the tree, so they need alignof(void *) alignment. > > The mentioned unaligned accesses were revealed by clang undefined > behaviour sanitizer, and are fixed by this patch. > > Part of #4609 Reviewed-by: Cyrill Gorcunov