Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, korablev@tarantool.org,
	tsafin@tarantool.org, alyapunov@tarantool.org,
	gorcunov@gmail.com
Subject: [Tarantool-patches] [PATCH 08/10] tuple: use unaligned store-load for field map
Date: Thu, 21 May 2020 22:37:31 +0200	[thread overview]
Message-ID: <070de438e70c57b6177f5257a46b57151b519ca5.1590093222.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1590093222.git.v.shpilevoy@tarantool.org>

A tuple can have a field map. It is an array of uint32_t values,
stored right after 'struct tuple' object in the same memory block.

'struct tuple' is 10 byte size, and is aligned by 4 bytes (even
though it is 'PACKED', so does not need an alignment). It means,
that field map is stored by an address like this: 4*N + 10.

This address is never aligned by 4, needed for uint32_t field map
array. So the array members should be accessed using operations
aware of unaligned nature of the addresses.

Unfortunately, it is impossible to make the field map aligned,
because that requires + 2 bytes for each tuple. It is unaffordable
luxury, since tuple count can be millions and even billions. Every
new byte may result in gigabytes of memory in a cluster.

The patch makes field map accessed using unaligned store-load
operations.

Part of #4609
---
 src/box/field_map.c | 13 +++++++++----
 src/box/field_map.h | 21 ++++++++++++---------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/box/field_map.c b/src/box/field_map.c
index 5f4661941..dc903115e 100644
--- a/src/box/field_map.c
+++ b/src/box/field_map.c
@@ -101,16 +101,21 @@ field_map_build(struct field_map_builder *builder, char *buffer)
 		(uint32_t *)(buffer + field_map_build_size(builder));
 	char *extent_wptr = buffer;
 	for (int32_t i = -1; i >= -(int32_t)builder->slot_count; i--) {
+		/*
+		 * Can not access field_map as a normal uint32
+		 * array because its alignment may be < 4 bytes.
+		 * Need to use unaligned store-load operations
+		 * explicitly.
+		 */
 		if (!builder->slots[i].has_extent) {
-			field_map[i] = builder->slots[i].offset;
+			store_u32(&field_map[i], builder->slots[i].offset);
 			continue;
 		}
 		struct field_map_builder_slot_extent *extent =
 						builder->slots[i].extent;
 		/** Retrive memory for the extent. */
-		field_map[i] =
-			(uint32_t)((char *)extent_wptr - (char *)field_map);
-		*(uint32_t *)extent_wptr = extent->size;
+		store_u32(&field_map[i], extent_wptr - (char *)field_map);
+		store_u32(extent_wptr, extent->size);
 		uint32_t extent_offset_sz = extent->size * sizeof(uint32_t);
 		memcpy(&((uint32_t *) extent_wptr)[1], extent->offset,
 			extent_offset_sz);
diff --git a/src/box/field_map.h b/src/box/field_map.h
index a1a5a9dba..d8ef726a1 100644
--- a/src/box/field_map.h
+++ b/src/box/field_map.h
@@ -33,6 +33,7 @@
 #include <assert.h>
 #include <stdint.h>
 #include <stddef.h>
+#include "bit/bit.h"
 
 struct region;
 struct field_map_builder_slot;
@@ -151,20 +152,22 @@ static inline uint32_t
 field_map_get_offset(const uint32_t *field_map, int32_t offset_slot,
 		     int multikey_idx)
 {
-	uint32_t offset;
-	if (multikey_idx != MULTIKEY_NONE &&
-	    (int32_t) field_map[offset_slot] < 0) {
+	/*
+	 * Can not access field_map as a normal uint32 array
+	 * because its alignment may be < 4 bytes. Need to use
+	 * unaligned store-load operations explicitly.
+	 */
+	uint32_t offset = load_u32(&field_map[offset_slot]);
+	if (multikey_idx != MULTIKEY_NONE && (int32_t)offset < 0) {
 		/**
 		 * The field_map extent has the following
 		 * structure: [size=N|slot1|slot2|..|slotN]
 		 */
-		uint32_t *extent = (uint32_t *)((char *)field_map +
-					(int32_t)field_map[offset_slot]);
-		if ((uint32_t)multikey_idx >= extent[0])
+		const uint32_t *extent = (const uint32_t *)
+			((const char *)field_map + (int32_t)offset);
+		if ((uint32_t)multikey_idx >= load_u32(&extent[0]))
 			return 0;
-		offset = extent[multikey_idx + 1];
-	} else {
-		offset = field_map[offset_slot];
+		offset = load_u32(&extent[multikey_idx + 1]);
 	}
 	return offset;
 }
-- 
2.21.1 (Apple Git-122.3)

  parent reply	other threads:[~2020-05-21 20:37 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21 20:37 [Tarantool-patches] [PATCH 00/10] Sanitize unaligned access Vladislav Shpilevoy
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 01/10] small: sanitized rlist and new region API Vladislav Shpilevoy
2020-06-08 12:17   ` Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 10/10] xrow: use unaligned store operation in xrow_to_iovec() Vladislav Shpilevoy
2020-06-08 12:26   ` Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 02/10] cmake: ignore warnings on alignof() and offsetof() Vladislav Shpilevoy
2020-06-08 12:52   ` Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 03/10] cmake: add option ENABLE_UB_SANITIZER Vladislav Shpilevoy
2020-06-08 12:53   ` Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 04/10] crc32: disable align sanitizer Vladislav Shpilevoy
2020-06-08 13:58   ` Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 05/10] sql: make BtCursor's memory aligned Vladislav Shpilevoy
2020-06-08 13:58   ` Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 06/10] region: use aligned allocations where necessary Vladislav Shpilevoy
2020-06-08 14:00   ` Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 07/10] vinyl: align statements and bps tree extents Vladislav Shpilevoy
2020-06-08 14:02   ` Cyrill Gorcunov
2020-05-21 20:37 ` Vladislav Shpilevoy [this message]
2020-06-08 14:04   ` [Tarantool-patches] [PATCH 08/10] tuple: use unaligned store-load for field map Cyrill Gorcunov
2020-05-21 20:37 ` [Tarantool-patches] [PATCH 09/10] port: make port_c_entry not PACKED Vladislav Shpilevoy
2020-06-08 14:04   ` Cyrill Gorcunov
2020-05-21 22:25 ` [Tarantool-patches] [PATCH 00/10] Sanitize unaligned access Sergey Bronnikov
2020-05-27 23:33   ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=070de438e70c57b6177f5257a46b57151b519ca5.1590093222.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=alyapunov@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=tsafin@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 08/10] tuple: use unaligned store-load for field map' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox