[PATCH v2 4/4] Allow to reuse tuple_formats for ephemeral spaces
Vladimir Davydov
vdavydov.dev at gmail.com
Thu Jan 24 20:49:05 MSK 2019
On Thu, Jan 24, 2019 at 03:48:48PM +0300, Kirill Yukhin wrote:
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index 4aad8da..cbc5091 100644
> --- a/src/box/memtx_space.c
> +++ b/src/box/memtx_space.c
> @@ -993,7 +993,7 @@ memtx_space_new(struct memtx_engine *memtx,
> tuple_format_new(&memtx_tuple_format_vtab, memtx, keys, key_count,
> def->fields, def->field_count,
> def->exact_field_count, def->dict,
> - def->opts.is_temporary);
> + def->opts.is_temporary, def->opts.is_temporary);
is_temporary, is_temporary WTF?
> if (format == NULL) {
> free(memtx_space);
> return NULL;
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 559c601..1e8675c 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -380,6 +387,69 @@ error:
> return NULL;
> }
>
> +static int
> +tuple_format_cmp(const struct tuple_format *a, const struct tuple_format *b)
> +{
> + if (a->exact_field_count != b->exact_field_count)
> + return a->exact_field_count - b->exact_field_count;
> + if (tuple_format_field_count(a) != tuple_format_field_count(b))
> + return tuple_format_field_count(a) - tuple_format_field_count(b);
> +
> + for (uint32_t i = 0; i < tuple_format_field_count((struct tuple_format *)a); ++i) {
^^^^^^^^^^^^^^^^^^^^^^^^
You forgot to drop this type conversion.
> + struct tuple_field *field_a = tuple_format_field(
> + (struct tuple_format *)a, i);
> + struct tuple_field *field_b = tuple_format_field(
> + (struct tuple_format *)b, i);
> + if (field_a->type != field_b->type)
> + return (int)field_a->type - (int)field_b->type;
> + if (field_a->coll_id != field_b->coll_id)
> + return (int)field_a->coll_id - (int)field_b->coll_id;
> + if (field_a->nullable_action != field_b->nullable_action)
> + return (int)field_a->nullable_action -
> + (int)field_b->nullable_action;
> + if (field_a->is_key_part != field_b->is_key_part)
> + return (int)field_a->is_key_part -
> + (int)field_b->is_key_part;
> + }
> +
> + return 0;
> +}
> +
> +static uint32_t
> +tuple_format_hash(struct tuple_format *format)
> +{
> +#define TUPLE_FIELD_MEMBER_HASH(field, member, h, carry, size) \
> + PMurHash32_Process(&h, &carry, &field->member, \
> + sizeof(field->member)); \
> + size += sizeof(field->member);
> +
> + uint32_t h = 13;
> + uint32_t carry = 0;
> + uint32_t size = 0;
> + for (uint32_t i = 0; i < tuple_format_field_count(format); ++i) {
> + struct tuple_field *f = tuple_format_field(format, i);
> + TUPLE_FIELD_MEMBER_HASH(f, type, h, carry, size)
> + TUPLE_FIELD_MEMBER_HASH(f, coll_id, h, carry, size)
> + TUPLE_FIELD_MEMBER_HASH(f, nullable_action, h, carry, size)
> + TUPLE_FIELD_MEMBER_HASH(f, is_key_part, h, carry, size)
> + }
> +#undef TUPLE_FIELD_MEMBER_HASH
> + return PMurHash32_Result(h, carry, size);
> +}
> +
> +#define MH_SOURCE 1
> +#define mh_name _tuple_format
> +#define mh_key_t struct tuple_format *
> +#define mh_node_t struct tuple_format *
> +#define mh_arg_t void *
> +#define mh_hash(a, arg) ((*(a))->hash)
> +#define mh_hash_key(a, arg) ((a)->hash)
> +#define mh_cmp(a, b, arg) (tuple_format_cmp(*(a), *(b)))
> +#define mh_cmp_key(a, b, arg) (tuple_format_cmp((a), *(b)))
> +#include "salad/mhash.h"
> +
> +struct mh_tuple_format_t *tuple_formats_hash = NULL;
> +
> /** Free tuple format resources, doesn't unregister. */
> static inline void
> tuple_format_destroy(struct tuple_format *format)
> @@ -392,17 +462,82 @@ tuple_format_destroy(struct tuple_format *format)
> void
> tuple_format_delete(struct tuple_format *format)
> {
> + mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format, NULL);
> + if (key != mh_end(tuple_formats_hash))
> + mh_tuple_format_del(tuple_formats_hash, key, NULL);
Please factor this out to a separate function complimentary to
tuple_format_add_to_hash (tuple_format_remove_from_hash?).
> tuple_format_deregister(format);
> tuple_format_destroy(format);
> free(format);
> }
>
> +/**
> + * Try to reuse given format. This is only possible for formats
> + * of ephemeral spaces, since we need to be sure that shared
> + * dictionary will never be altered. If it can, then alter can
> + * affect another space, which shares a format with one which is
> + * altered.
> + * @param p_format Double pointer to format. It is updated with
> + * hashed value, if corresponding format was found
> + * in hash table
> + * @retval Returns true if format was found in hash table, false
> + * otherwise.
> + *
> + */
> +static bool
> +tuple_format_reuse(struct tuple_format **p_format)
> +{
> + struct tuple_format *format = *p_format;
> + if (!format->is_ephemeral)
> + return false;
> + /*
> + * These fields do not participate in hashing.
> + * Make sure they're unset.
> + */
> + assert(format->dict->name_count == 0);
> + assert(format->is_temporary);
> + format->hash = tuple_format_hash(format);
The fact that in tuple_format_add_to_hash you implicitly rely on
format->hash having been initialized here looks confusing. Let's
initialize hash for all formats.
> + mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format,
> + NULL);
> + if (key != mh_end(tuple_formats_hash)) {
> + struct tuple_format **entry = mh_tuple_format_node(
> + tuple_formats_hash, key);
> + tuple_format_destroy(format);
> + free(format);
> + *p_format = *entry;
> + return true;
> + }
> + return false;
> +}
> +
> +/**
> + * See justification, why ephemeral space's formats are
> + * only feasible for hasing.
> + * @retval 0 on success, even if format wasn't added to hash
> + * -1 in case of error.
> + */
> +static int
> +tuple_format_add_to_hash(struct tuple_format *format)
> +{
> + if(!format->is_ephemeral)
> + return 0;
> + assert(format->dict->name_count == 0);
> + assert(format->is_temporary);
> + mh_int_t key = mh_tuple_format_put(tuple_formats_hash,
> + (const struct tuple_format **)&format,
> + NULL, NULL);
> + if (key == mh_end(tuple_formats_hash))
You forgot to set diag (say hi to #3534 again ;-)
> + return -1;
> + else
> + return 0;
> +}
> +
> struct tuple_format *
> tuple_format_new(struct tuple_format_vtab *vtab, void *engine,
> struct key_def * const *keys, uint16_t key_count,
> const struct field_def *space_fields,
> uint32_t space_field_count, uint32_t exact_field_count,
> - struct tuple_dictionary *dict, bool is_temporary)
> + struct tuple_dictionary *dict, bool is_temporary,
> + bool is_ephemeral)
> {
> struct tuple_format *format =
> tuple_format_alloc(keys, key_count, space_field_count, dict);
> diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
> index d15fef2..3fc3a23 100644
> --- a/src/box/tuple_format.h
> +++ b/src/box/tuple_format.h
> @@ -459,6 +470,13 @@ tuple_field_by_part_raw(struct tuple_format *format, const char *data,
> return tuple_field_raw(format, data, field_map, part->fieldno);
> }
>
> +/**
> + * Initialize tuple format subsystem.
> + * @retval 0 on success, -1 otherwise.
> + */
> +int
> +tuple_format_init();
> +
tuple_format_init is declared in the very end of the file while
tuple_format_free in the very beginning. Urghhh. Please fix.
> #if defined(__cplusplus)
> } /* extern "C" */
> #endif /* defined(__cplusplus) */
> diff --git a/test/unit/tuple_bigref.c b/test/unit/tuple_bigref.c
> index 20eab61..76a2e98 100644
> --- a/test/unit/tuple_bigref.c
> +++ b/test/unit/tuple_bigref.c
> @@ -143,6 +143,7 @@ main()
>
> memory_init();
> fiber_init(fiber_c_invoke);
> + tuple_format_init();
This is redundant. Remove please.
> tuple_init(NULL);
>
> tuple_end = mp_encode_array(tuple_end, 1);
> diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
> index 6808ff4..395ad15 100644
> --- a/test/unit/vy_iterators_helper.c
> +++ b/test/unit/vy_iterators_helper.c
> @@ -17,12 +17,13 @@ vy_iterator_C_test_init(size_t cache_size)
> say_set_log_level(S_WARN);
>
> memory_init();
> + tuple_format_init();
Ditto.
> fiber_init(fiber_c_invoke);
> 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, NULL, 0,
> - NULL, 0, 0, NULL, false);
> + NULL, 0, 0, NULL, false, false);
> tuple_format_ref(vy_key_format);
>
> size_t mem_size = 64 * 1024 * 1024;
More information about the Tarantool-patches
mailing list