From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 8/8] tuple: zap tuple_extra Date: Sun, 14 Oct 2018 21:16:52 +0300 Message-Id: <137ff9cb723921ebcaa4af3455b2466866f17047.1539539421.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: tuple_extra() allows to store arbitrary metadata inside tuples. To use it, one should set extra_size when creating a tuple_format. It was introduced for storing UPSERT counter or column mask inside vinyl statements. Turned out that it wasn't really needed as UPSERT counter can be stored on lsregion while column mask doesn't need to be stored at all. Actually, the whole idea of tuple_extra() is rather crooked: why would we need it if we can inherit struct tuple instead, as we do in case of memtx_tuple and vy_stmt? Accessing an inherited struct is much more convenient than using tuple_extra(). So this patch gets rid of tuple_extra(). To do that, it partially reverts the following commits: 6c0842e0302c vinyl: refactor vy_stmt_alloc() 74ff46d8e93d vinyl: add special format for tuples with column mask 11eb7816a30e Add extra size to tuple_format->field_map_size --- src/box/blackhole.c | 2 +- src/box/memtx_engine.c | 10 +++++----- src/box/memtx_space.c | 2 +- src/box/tuple.c | 12 ++++++------ src/box/tuple.h | 30 ++++++------------------------ src/box/tuple_format.c | 6 ++---- src/box/tuple_format.h | 23 +---------------------- src/box/vinyl.c | 4 ++-- src/box/vy_lsm.c | 5 ++--- src/box/vy_stmt.c | 8 ++++---- test/unit/vy_iterators_helper.c | 6 +++--- test/unit/vy_mem.c | 2 +- test/unit/vy_point_lookup.c | 2 +- 13 files changed, 35 insertions(+), 77 deletions(-) diff --git a/src/box/blackhole.c b/src/box/blackhole.c index f979304e..08cbe77f 100644 --- a/src/box/blackhole.c +++ b/src/box/blackhole.c @@ -151,7 +151,7 @@ blackhole_engine_create_space(struct engine *engine, struct space_def *def, /* Allocate tuples on runtime arena, but check space format. */ struct tuple_format *format; - format = tuple_format_new(&tuple_format_runtime->vtab, NULL, 0, 0, + format = tuple_format_new(&tuple_format_runtime->vtab, NULL, 0, def->fields, def->field_count, def->dict); if (format == NULL) { free(space); diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c index 5a5e87e6..cda578e1 100644 --- a/src/box/memtx_engine.c +++ b/src/box/memtx_engine.c @@ -1130,8 +1130,8 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end) struct memtx_engine *memtx = (struct memtx_engine *)format->engine; assert(mp_typeof(*data) == MP_ARRAY); size_t tuple_len = end - data; - size_t meta_size = tuple_format_meta_size(format); - size_t total = sizeof(struct memtx_tuple) + meta_size + tuple_len; + size_t total = sizeof(struct memtx_tuple) + format->field_map_size + + tuple_len; ERROR_INJECT(ERRINJ_TUPLE_ALLOC, { diag_set(OutOfMemory, total, "slab allocator", "memtx_tuple"); @@ -1166,7 +1166,7 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end) * tuple base, not from memtx_tuple, because the struct * tuple is not the first field of the memtx_tuple. */ - tuple->data_offset = sizeof(struct tuple) + meta_size; + tuple->data_offset = sizeof(struct tuple) + format->field_map_size; char *raw = (char *) tuple + tuple->data_offset; uint32_t *field_map = (uint32_t *) raw; memcpy(raw, data, tuple_len); @@ -1184,8 +1184,8 @@ memtx_tuple_delete(struct tuple_format *format, struct tuple *tuple) struct memtx_engine *memtx = (struct memtx_engine *)format->engine; say_debug("%s(%p)", __func__, tuple); assert(tuple->refs == 0); - size_t total = sizeof(struct memtx_tuple) + - tuple_format_meta_size(format) + tuple->bsize; + size_t total = sizeof(struct memtx_tuple) + format->field_map_size + + tuple->bsize; tuple_format_unref(format); struct memtx_tuple *memtx_tuple = container_of(tuple, struct memtx_tuple, base); diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c index 08ae0daa..0ff89d7e 100644 --- a/src/box/memtx_space.c +++ b/src/box/memtx_space.c @@ -909,7 +909,7 @@ memtx_space_new(struct memtx_engine *memtx, keys[key_count++] = index_def->key_def; struct tuple_format *format = - tuple_format_new(&memtx_tuple_format_vtab, keys, key_count, 0, + tuple_format_new(&memtx_tuple_format_vtab, keys, key_count, def->fields, def->field_count, def->dict); if (format == NULL) { free(memtx_space); diff --git a/src/box/tuple.c b/src/box/tuple.c index c975e539..0bf4c61e 100644 --- a/src/box/tuple.c +++ b/src/box/tuple.c @@ -97,8 +97,8 @@ runtime_tuple_new(struct tuple_format *format, const char *data, const char *end mp_tuple_assert(data, end); size_t data_len = end - data; - size_t meta_size = tuple_format_meta_size(format); - size_t total = sizeof(struct tuple) + meta_size + data_len; + size_t total = sizeof(struct tuple) + format->field_map_size + + data_len; struct tuple *tuple = (struct tuple *) smalloc(&runtime_alloc, total); if (tuple == NULL) { @@ -111,7 +111,7 @@ runtime_tuple_new(struct tuple_format *format, const char *data, const char *end tuple->bsize = data_len; tuple->format_id = tuple_format_id(format); tuple_format_ref(format); - tuple->data_offset = sizeof(struct tuple) + meta_size; + tuple->data_offset = sizeof(struct tuple) + format->field_map_size; char *raw = (char *) tuple + tuple->data_offset; uint32_t *field_map = (uint32_t *) raw; memcpy(raw, data, data_len); @@ -129,7 +129,7 @@ runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple) assert(format->vtab.tuple_delete == tuple_format_runtime_vtab.tuple_delete); say_debug("%s(%p)", __func__, tuple); assert(tuple->refs == 0); - size_t total = sizeof(struct tuple) + tuple_format_meta_size(format) + + size_t total = sizeof(struct tuple) + format->field_map_size + tuple->bsize; tuple_format_unref(format); smfree(&runtime_alloc, tuple, total); @@ -223,7 +223,7 @@ tuple_init(field_name_hash_f hash) * Create a format for runtime tuples */ tuple_format_runtime = tuple_format_new(&tuple_format_runtime_vtab, - NULL, 0, 0, NULL, 0, NULL); + NULL, 0, NULL, 0, NULL); if (tuple_format_runtime == NULL) return -1; @@ -395,7 +395,7 @@ box_tuple_format_new(struct key_def **keys, uint16_t key_count) { box_tuple_format_t *format = tuple_format_new(&tuple_format_runtime_vtab, - keys, key_count, 0, NULL, 0, NULL); + keys, key_count, NULL, 0, NULL); if (format != NULL) tuple_format_ref(format); return format; diff --git a/src/box/tuple.h b/src/box/tuple.h index b638f508..5c6dc6b8 100644 --- a/src/box/tuple.h +++ b/src/box/tuple.h @@ -289,18 +289,12 @@ box_tuple_upsert(const box_tuple_t *tuple, const char *expr, const /** * An atom of Tarantool storage. Represents MsgPack Array. * Tuple has the following structure: - * format->tuple_meta_size bsize - * +------------------------+-------------+ - * tuple_begin, ..., raw = | tuple_meta | MessagePack | - * | +------------------------+-------------+ - * | ^ - * +---------------------------------------------data_offset - * - * tuple_meta structure: - * +----------------------+-----------------------+ - * | extra_size | offset N ... offset 1 | - * +----------------------+-----------------------+ - * @sa tuple_format_new() uint32 ... uint32 + * uint32 uint32 bsize + * +-------------------+-------------+ + * tuple_begin, ..., raw = | offN | ... | off1 | MessagePack | + * | +-------------------+-------------+ + * | ^ + * +---------------------------------------data_offset * * Each 'off_i' is the offset to the i-th indexed field. */ @@ -416,18 +410,6 @@ tuple_format(const struct tuple *tuple) } /** - * Return extra data saved in tuple metadata. - * @param tuple tuple - * @return a pointer to extra data saved in tuple metadata. - */ -static inline const char * -tuple_extra(const struct tuple *tuple) -{ - struct tuple_format *format = tuple_format(tuple); - return tuple_data(tuple) - tuple_format_meta_size(format); -} - -/** * Instantiate a new engine-independent tuple from raw MsgPack Array data * using runtime arena. Use this function to create a standalone tuple * from Lua or C procedures. diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c index c510ffd9..1c4b2e6a 100644 --- a/src/box/tuple_format.c +++ b/src/box/tuple_format.c @@ -140,7 +140,7 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys, assert(format->fields[0].offset_slot == TUPLE_OFFSET_SLOT_NIL); size_t field_map_size = -current_slot * sizeof(uint32_t); - if (field_map_size + format->extra_size > UINT16_MAX) { + if (field_map_size > UINT16_MAX) { /** tuple->data_offset is 16 bits */ diag_set(ClientError, ER_INDEX_FIELD_COUNT_LIMIT, -current_slot); @@ -258,8 +258,7 @@ tuple_format_delete(struct tuple_format *format) struct tuple_format * tuple_format_new(struct tuple_format_vtab *vtab, struct key_def * const *keys, - uint16_t key_count, uint16_t extra_size, - const struct field_def *space_fields, + uint16_t key_count, const struct field_def *space_fields, uint32_t space_field_count, struct tuple_dictionary *dict) { struct tuple_format *format = @@ -268,7 +267,6 @@ tuple_format_new(struct tuple_format_vtab *vtab, struct key_def * const *keys, return NULL; format->vtab = *vtab; format->engine = NULL; - format->extra_size = extra_size; format->is_temporary = false; if (tuple_format_register(format) < 0) { tuple_format_destroy(format); diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h index cbd4e4b4..345e41b0 100644 --- a/src/box/tuple_format.h +++ b/src/box/tuple_format.h @@ -130,12 +130,6 @@ struct tuple_format { */ bool is_temporary; /** - * The number of extra bytes to reserve in tuples before - * field map. - * \sa struct tuple - */ - uint16_t extra_size; - /** * Size of field map of tuple in bytes. * \sa struct tuple */ @@ -204,7 +198,6 @@ tuple_format_unref(struct tuple_format *format) * @param vtab Virtual function table for specific engines. * @param keys Array of key_defs of a space. * @param key_count The number of keys in @a keys array. - * @param extra_size Extra bytes to reserve in tuples metadata. * @param space_fields Array of fields, defined in a space format. * @param space_field_count Length of @a space_fields. * @@ -213,8 +206,7 @@ tuple_format_unref(struct tuple_format *format) */ struct tuple_format * tuple_format_new(struct tuple_format_vtab *vtab, struct key_def * const *keys, - uint16_t key_count, uint16_t extra_size, - const struct field_def *space_fields, + uint16_t key_count, const struct field_def *space_fields, uint32_t space_field_count, struct tuple_dictionary *dict); /** @@ -233,19 +225,6 @@ tuple_format1_can_store_format2_tuples(const struct tuple_format *format1, const struct tuple_format *format2); /** - * Returns the total size of tuple metadata of this format. - * See @link struct tuple @endlink for explanation of tuple layout. - * - * @param format Tuple Format. - * @returns the total size of tuple metadata - */ -static inline uint16_t -tuple_format_meta_size(const struct tuple_format *format) -{ - return format->extra_size + format->field_map_size; -} - -/** * Calculate minimal field count of tuples with specified keys and * space format. * @param keys Array of key definitions of indexes. diff --git a/src/box/vinyl.c b/src/box/vinyl.c index c1e70cb2..e4d97c9a 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -603,7 +603,7 @@ vinyl_engine_create_space(struct engine *engine, struct space_def *def, keys[key_count++] = index_def->key_def; struct tuple_format *format = - tuple_format_new(&vy_tuple_format_vtab, keys, key_count, 0, + tuple_format_new(&vy_tuple_format_vtab, keys, key_count, def->fields, def->field_count, def->dict); if (format == NULL) { free(space); @@ -3007,7 +3007,7 @@ vy_send_lsm(struct vy_join_ctx *ctx, struct vy_lsm_recovery_info *lsm_info) if (ctx->key_def == NULL) goto out; ctx->format = tuple_format_new(&vy_tuple_format_vtab, &ctx->key_def, - 1, 0, NULL, 0, NULL); + 1, NULL, 0, NULL); if (ctx->format == NULL) goto out_free_key_def; tuple_format_ref(ctx->format); diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c index 80341267..e024e508 100644 --- a/src/box/vy_lsm.c +++ b/src/box/vy_lsm.c @@ -61,7 +61,7 @@ vy_lsm_env_create(struct vy_lsm_env *env, const char *path, void *upsert_thresh_arg) { env->key_format = tuple_format_new(&vy_tuple_format_vtab, - NULL, 0, 0, NULL, 0, NULL); + NULL, 0, NULL, 0, NULL); if (env->key_format == NULL) return -1; tuple_format_ref(env->key_format); @@ -154,8 +154,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env, lsm->disk_format = format; } else { lsm->disk_format = tuple_format_new(&vy_tuple_format_vtab, - &cmp_def, 1, 0, NULL, 0, - NULL); + &cmp_def, 1, NULL, 0, NULL); if (lsm->disk_format == NULL) goto fail_format; } diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c index 7082952b..d8384040 100644 --- a/src/box/vy_stmt.c +++ b/src/box/vy_stmt.c @@ -127,8 +127,8 @@ vy_tuple_delete(struct tuple_format *format, struct tuple *tuple) static struct tuple * vy_stmt_alloc(struct tuple_format *format, uint32_t bsize) { - uint32_t meta_size = tuple_format_meta_size(format); - uint32_t total_size = sizeof(struct vy_stmt) + meta_size + bsize; + uint32_t total_size = sizeof(struct vy_stmt) + format->field_map_size + + bsize; if (unlikely(total_size > vy_max_tuple_size)) { diag_set(ClientError, ER_VINYL_MAX_TUPLE_SIZE, (unsigned) total_size); @@ -141,13 +141,13 @@ vy_stmt_alloc(struct tuple_format *format, uint32_t bsize) return NULL; } say_debug("vy_stmt_alloc(format = %d %u, bsize = %zu) = %p", - format->id, tuple_format_meta_size(format), bsize, tuple); + format->id, format->field_map_size, bsize, tuple); tuple->refs = 1; tuple->format_id = tuple_format_id(format); if (cord_is_main()) tuple_format_ref(format); tuple->bsize = bsize; - tuple->data_offset = sizeof(struct vy_stmt) + meta_size;; + tuple->data_offset = sizeof(struct vy_stmt) + format->field_map_size; vy_stmt_set_lsn(tuple, 0); vy_stmt_set_type(tuple, 0); vy_stmt_set_flags(tuple, 0); diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c index dcd76e4c..7fad5600 100644 --- a/test/unit/vy_iterators_helper.c +++ b/test/unit/vy_iterators_helper.c @@ -21,7 +21,7 @@ vy_iterator_C_test_init(size_t cache_size) tuple_init(NULL); vy_cache_env_create(&cache_env, cord_slab_cache()); vy_cache_env_set_quota(&cache_env, cache_size); - vy_key_format = tuple_format_new(&vy_tuple_format_vtab, NULL, 0, 0, + vy_key_format = tuple_format_new(&vy_tuple_format_vtab, NULL, 0, NULL, 0, NULL); tuple_format_ref(vy_key_format); @@ -202,7 +202,7 @@ create_test_mem(struct key_def *def) struct key_def * const defs[] = { def }; struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab, defs, def->part_count, - 0, NULL, 0, NULL); + NULL, 0, NULL); fail_if(format == NULL); /* Create mem */ @@ -219,7 +219,7 @@ create_test_cache(uint32_t *fields, uint32_t *types, *def = box_key_def_new(fields, types, key_cnt); assert(*def != NULL); vy_cache_create(cache, &cache_env, *def, true); - *format = tuple_format_new(&vy_tuple_format_vtab, def, 1, 0, NULL, 0, + *format = tuple_format_new(&vy_tuple_format_vtab, def, 1, NULL, 0, NULL); tuple_format_ref(*format); } diff --git a/test/unit/vy_mem.c b/test/unit/vy_mem.c index 967aabe8..ebf3fbc7 100644 --- a/test/unit/vy_mem.c +++ b/test/unit/vy_mem.c @@ -77,7 +77,7 @@ test_iterator_restore_after_insertion() /* Create format */ struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab, - &key_def, 1, 0, NULL, 0, + &key_def, 1, NULL, 0, NULL); assert(format != NULL); tuple_format_ref(format); diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c index 8cf0fb72..65dafcb2 100644 --- a/test/unit/vy_point_lookup.c +++ b/test/unit/vy_point_lookup.c @@ -84,7 +84,7 @@ test_basic() vy_cache_create(&cache, &cache_env, key_def, true); struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab, - &key_def, 1, 0, NULL, 0, + &key_def, 1, NULL, 0, NULL); isnt(format, NULL, "tuple_format_new is not NULL"); tuple_format_ref(format); -- 2.11.0