commit e06b866a738a376b3f75017e7cd76811a5eb594d
from: Vladimir Davydov <vdavydov@tarantool.org>
via: Vladimir Davydov <vdavydov.dev@gmail.com>
date: Wed Jan 17 13:51:55 2024 UTC

space: optimize lookup by id

In contrast to space_cache_find(), space_by_id() doesn't cache the last
looked up space to speed up following lookups. This is confusing,
because one would expect the only difference between space_cache_find()
and space_by_id() functions to be that the former sets diag while the
latter doesn't. Let's move the prev_space cache to space_by_id() to fix
this issue.

To achieve that we need to make space_by_id() inline, which is tricky
because it's called via FFI. We work around that by defining a macro
space_by_id() which expands to space_by_id_fast() while still keeping
the space_by_id() symbol for FFI.

Last but not least, we make the last cached space global (currently it's
instantiated once per each space_cache_find() call site). This is a good
thing because we often call space_by_id() a few times while processing
the same DML request (e.g. in memtx mvcc).

NO_DOC=no user-visible behavior changes
NO_TEST=no user-visible behavior changes
NO_CHANGELOG=no user-visible behavior changes

commit - 9ee99c5ff9b64d01be6bd3b425b442a0a3421fcf
commit + e06b866a738a376b3f75017e7cd76811a5eb594d
blob - e5a41478d3a0377cea133fe0ac9e193fe49b3f3d
blob + f9b2e1b5ba6a61b6d98b7686b525ec83b0f5c170
--- src/box/space_cache.c
+++ src/box/space_cache.c
@@ -22,6 +22,15 @@ static struct mh_strnptr_t *spaces_by_name;
  */
 uint32_t space_cache_version;
 
+/**
+ * Value of space_cache_version at the time of the last space lookup,
+ * see space_by_id_fast().
+ */
+uint32_t prev_space_cache_version;
+
+/** Last looked up space, see space_by_id_fast(). */
+struct space *prev_space;
+
 const char *space_cache_holder_type_strs[SPACE_HOLDER_MAX] = {
 	"foreign key",
 };
@@ -48,9 +57,8 @@ space_cache_destroy(void)
 	mh_strnptr_delete(spaces_by_name);
 }
 
-/** Return space by its number */
 struct space *
-space_by_id(uint32_t id)
+space_by_id_slow(uint32_t id)
 {
 	mh_int_t space = mh_i32ptr_find(spaces, id, NULL);
 	if (space == mh_end(spaces))
@@ -250,3 +258,12 @@ space_cache_is_pinned(struct space *space, enum space_
 	}
 	return false;
 }
+
+#undef space_by_id
+
+/** Define the space_by_id() symbol for FFI. */
+struct space *
+space_by_id(uint32_t id)
+{
+	return space_by_id_fast(id);
+}
blob - 97d64b52b1bf2294777699449d5c464659b0b258
blob + 86ed250a205c5fe97af4f5f47ec3d8ae045fe7e8
--- src/box/space_cache.h
+++ src/box/space_cache.h
@@ -81,6 +81,32 @@ void
 space_cache_destroy(void);
 
 /**
+ * Slow version of space lookup by id.
+ * Performs a direct lookup in the spaces hash table.
+ * Returns NULL if not found (doesn't set diag).
+ */
+struct space *
+space_by_id_slow(uint32_t id);
+
+/**
+ * Fast version of space lookup by id.
+ * Caches the last looked up space.
+ * Returns NULL if not found (doesn't set diag).
+ */
+static inline struct space *
+space_by_id_fast(uint32_t id)
+{
+	extern uint32_t prev_space_cache_version;
+	extern struct space *prev_space;
+	if (prev_space_cache_version == space_cache_version &&
+	    prev_space != NULL && prev_space->def->id == id)
+		return prev_space;
+	prev_space = space_by_id_slow(id);
+	prev_space_cache_version = space_cache_version;
+	return prev_space;
+}
+
+/**
  * Try to look up a space by space number in the space cache.
  * FFI-friendly no-exception-thrown space lookup function.
  *
@@ -89,6 +115,12 @@ space_cache_destroy(void);
 struct space *
 space_by_id(uint32_t id);
 
+/*
+ * Use the inline function for space lookups in Tarantool.
+ * The space_by_id() symbol exists only for FFI.
+ */
+#define space_by_id(id) space_by_id_fast(id)
+
 /**
  * Try to look up a space by space name in the space name cache.
  *
@@ -119,17 +151,9 @@ space_cache_find_next_unused_id(uint32_t cur_id);
 static inline struct space *
 space_cache_find(uint32_t id)
 {
-	static uint32_t prev_space_cache_version;
-	static struct space *space;
-	if (prev_space_cache_version != space_cache_version)
-		space = NULL;
-	if (space && space->def->id == id)
+	struct space *space = space_by_id(id);
+	if (space != NULL)
 		return space;
-	space = space_by_id(id);
-	if (space != NULL) {
-		prev_space_cache_version = space_cache_version;
-		return space;
-	}
 	diag_set(ClientError, ER_NO_SUCH_SPACE, int2str(id));
 	return NULL;
 }
blob - 59ceb6c5691b0fbd9dd9e83c38af46e435238ec4
blob + ab7058df77339f8cc2fe48fa52a2658ad2cf5203
--- test/unit/vy_point_lookup.c
+++ test/unit/vy_point_lookup.c
@@ -12,7 +12,10 @@
 
 uint64_t schema_version;
 uint32_t space_cache_version;
-struct space *space_by_id(uint32_t id) { return NULL; }
+uint32_t prev_space_cache_version;
+struct space *prev_space;
+struct space *
+space_by_id_slow(uint32_t id) { return NULL; }
 struct vy_lsm *vy_lsm(struct index *index) { return NULL; }
 void index_delete(struct index *index) { unreachable(); }