From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com Subject: [PATCH 1/5] tuple: remove alloc and alloc_ctx args from update() Date: Sun, 14 Jul 2019 00:11:04 +0200 [thread overview] Message-ID: <01397cda246da9b8dc54e7c5fbf92b38dfd1fdee.1563054879.git.v.shpilevoy@tarantool.org> (raw) In-Reply-To: <cover.1563054879.git.v.shpilevoy@tarantool.org> They are always region_aligned_alloc and region pointer. Lets use them always inside tuple_update.c, with no necessity to pass explicitly. The patch is motivated by forthcoming updates by JSON path, which will strongly complicate and perhaps slow down the tuple_update.c code. The present patch as well as some next ones should smooth these problems. --- src/box/lua/tuple.c | 9 +++-- src/box/memtx_space.c | 18 ++++------ src/box/space.c | 23 ++++++------- src/box/tuple.c | 6 ++-- src/box/tuple_update.c | 74 +++++++++++++++++++++-------------------- src/box/tuple_update.h | 14 +++----- src/box/vinyl.c | 10 ++---- src/box/vy_upsert.c | 29 ++++------------ test/unit/column_mask.c | 30 +++++++---------- 9 files changed, 88 insertions(+), 125 deletions(-) diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c index 183c3901d..2fbfb1473 100644 --- a/src/box/lua/tuple.c +++ b/src/box/lua/tuple.c @@ -447,11 +447,10 @@ lbox_tuple_transform(struct lua_State *L) * to use the default one with no restrictions on field * count or types. */ - const char *new_data = tuple_update_execute(region_aligned_alloc_cb, - region, buf->buf, - buf->buf + ibuf_used(buf), - old_data, old_data + bsize, - &new_size, 1, NULL); + const char *new_data = + tuple_update_execute(buf->buf, buf->buf + ibuf_used(buf), + old_data, old_data + bsize, &new_size, 1, + NULL); if (new_data != NULL) new_tuple = tuple_new(box_tuple_format_default(), new_data, new_data + new_size); diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c index f0e1cfd27..54b379c43 100644 --- a/src/box/memtx_space.c +++ b/src/box/memtx_space.c @@ -419,8 +419,7 @@ memtx_space_execute_update(struct space *space, struct txn *txn, uint32_t new_size = 0, bsize; const char *old_data = tuple_data_range(old_tuple, &bsize); const char *new_data = - tuple_update_execute(region_aligned_alloc_cb, &fiber()->gc, - request->tuple, request->tuple_end, + tuple_update_execute(request->tuple, request->tuple_end, old_data, old_data + bsize, &new_size, request->index_base, NULL); if (new_data == NULL) @@ -489,9 +488,8 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn, * we get it here, it's also OK to throw it * @sa https://github.com/tarantool/tarantool/issues/1156 */ - if (tuple_update_check_ops(region_aligned_alloc_cb, &fiber()->gc, - request->ops, request->ops_end, - request->index_base)) { + if (tuple_update_check_ops(request->ops, request->ops_end, + request->index_base) != 0) { return -1; } stmt->new_tuple = memtx_tuple_new(space->format, @@ -511,12 +509,10 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn, */ uint64_t column_mask = COLUMN_MASK_FULL; const char *new_data = - tuple_upsert_execute(region_aligned_alloc_cb, - &fiber()->gc, request->ops, - request->ops_end, old_data, - old_data + bsize, &new_size, - request->index_base, false, - &column_mask); + tuple_upsert_execute(request->ops, request->ops_end, + old_data, old_data + bsize, + &new_size, request->index_base, + false, &column_mask); if (new_data == NULL) return -1; diff --git a/src/box/space.c b/src/box/space.c index e7babb2ae..e881aaf32 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -292,7 +292,6 @@ static int space_before_replace(struct space *space, struct txn *txn, struct request *request) { - struct region *gc = &fiber()->gc; enum iproto_type type = request->type; struct index *pk = space_index(space, 0); @@ -358,10 +357,10 @@ space_before_replace(struct space *space, struct txn *txn, } old_data = tuple_data_range(old_tuple, &old_size); old_data_end = old_data + old_size; - new_data = tuple_update_execute(region_aligned_alloc_cb, gc, - request->tuple, request->tuple_end, - old_data, old_data_end, &new_size, - request->index_base, NULL); + new_data = tuple_update_execute(request->tuple, + request->tuple_end, old_data, + old_data_end, &new_size, + request->index_base, NULL); if (new_data == NULL) return -1; new_data_end = new_data + new_size; @@ -381,18 +380,18 @@ space_before_replace(struct space *space, struct txn *txn, */ new_data = request->tuple; new_data_end = request->tuple_end; - if (tuple_update_check_ops(region_aligned_alloc_cb, gc, - request->ops, request->ops_end, - request->index_base) != 0) + if (tuple_update_check_ops(request->ops, + request->ops_end, + request->index_base) != 0) return -1; break; } old_data = tuple_data_range(old_tuple, &old_size); old_data_end = old_data + old_size; - new_data = tuple_upsert_execute(region_aligned_alloc_cb, gc, - request->ops, request->ops_end, - old_data, old_data_end, &new_size, - request->index_base, false, NULL); + new_data = tuple_upsert_execute(request->ops, request->ops_end, + old_data, old_data_end, + &new_size, request->index_base, + false, NULL); new_data_end = new_data + new_size; break; default: diff --git a/src/box/tuple.c b/src/box/tuple.c index a7ef332b2..1e5b2fb24 100644 --- a/src/box/tuple.c +++ b/src/box/tuple.c @@ -706,8 +706,7 @@ box_tuple_update(box_tuple_t *tuple, const char *expr, const char *expr_end) struct region *region = &fiber()->gc; size_t used = region_used(region); const char *new_data = - tuple_update_execute(region_aligned_alloc_cb, region, expr, - expr_end, old_data, old_data + bsize, + tuple_update_execute(expr, expr_end, old_data, old_data + bsize, &new_size, 1, NULL); if (new_data == NULL) { region_truncate(region, used); @@ -729,8 +728,7 @@ box_tuple_upsert(box_tuple_t *tuple, const char *expr, const char *expr_end) struct region *region = &fiber()->gc; size_t used = region_used(region); const char *new_data = - tuple_upsert_execute(region_aligned_alloc_cb, region, expr, - expr_end, old_data, old_data + bsize, + tuple_upsert_execute(expr, expr_end, old_data, old_data + bsize, &new_size, 1, false, NULL); if (new_data == NULL) { region_truncate(region, used); diff --git a/src/box/tuple_update.c b/src/box/tuple_update.c index 7a203ced8..599952189 100644 --- a/src/box/tuple_update.c +++ b/src/box/tuple_update.c @@ -43,7 +43,7 @@ #include <bit/int96.h> #include <salad/rope.h> #include "column_mask.h" - +#include "fiber.h" /** UPDATE request implementation. * UPDATE request is represented by a sequence of operations, each @@ -96,10 +96,27 @@ * deleted one. */ +static inline void * +update_alloc(void *ctx, size_t size) +{ + void *ptr = region_aligned_alloc((struct region *) ctx, size, + alignof(uint64_t)); + if (ptr == NULL) + diag_set(OutOfMemory, size, "region_aligned_alloc", + "update internals"); + return ptr; +} + +static void +update_free(void *ctx, void *mem) +{ + (void) ctx; + (void) mem; +} + /** Update internal state */ struct tuple_update { - tuple_update_alloc_func alloc; void *alloc_ctx; struct rope *rope; struct update_op *ops; @@ -508,7 +525,7 @@ do_op_insert(struct tuple_update *update, struct update_op *op) if (op_adjust_field_no(update, op, rope_size(update->rope) + 1)) return -1; struct update_field *field = (struct update_field *) - update->alloc(update->alloc_ctx, sizeof(*field)); + update_alloc(update->alloc_ctx, sizeof(*field)); if (field == NULL) return -1; update_field_init(field, op->arg.set.value, op->arg.set.length, 0); @@ -778,7 +795,7 @@ update_field_split(void *split_ctx, void *data, size_t size, size_t offset) struct update_field *prev = (struct update_field *) data; struct update_field *next = (struct update_field *) - update->alloc(update->alloc_ctx, sizeof(*next)); + update_alloc(update->alloc_ctx, sizeof(*next)); if (next == NULL) return NULL; assert(offset > 0 && prev->tail_len > 0); @@ -799,14 +816,6 @@ update_field_split(void *split_ctx, void *data, size_t size, size_t offset) return next; } -/** Free rope node - do nothing, since we use a pool allocator. */ -static void -region_alloc_free_stub(void *ctx, void *mem) -{ - (void) ctx; - (void) mem; -} - /** * We found a tuple to do the update on. Prepare a rope * to perform operations on. @@ -822,14 +831,14 @@ int update_create_rope(struct tuple_update *update, const char *tuple_data, const char *tuple_data_end, uint32_t field_count) { - update->rope = rope_new(update_field_split, update, update->alloc, - region_alloc_free_stub, update->alloc_ctx); + update->rope = rope_new(update_field_split, update, update_alloc, + update_free, update->alloc_ctx); if (update->rope == NULL) return -1; /* Initialize the rope with the old tuple. */ struct update_field *first = (struct update_field *) - update->alloc(update->alloc_ctx, sizeof(*first)); + update_alloc(update->alloc_ctx, sizeof(*first)); if (first == NULL) return -1; const char *field = tuple_data; @@ -970,7 +979,7 @@ update_read_ops(struct tuple_update *update, const char *expr, } /* Read update operations. */ - update->ops = (struct update_op *) update->alloc(update->alloc_ctx, + update->ops = (struct update_op *) update_alloc(update->alloc_ctx, update->op_count * sizeof(struct update_op)); if (update->ops == NULL) return -1; @@ -1160,13 +1169,10 @@ upsert_do_ops(struct tuple_update *update, const char *old_data, } static void -update_init(struct tuple_update *update, - tuple_update_alloc_func alloc, void *alloc_ctx, - int index_base) +update_init(struct tuple_update *update, int index_base) { memset(update, 0, sizeof(*update)); - update->alloc = alloc; - update->alloc_ctx = alloc_ctx; + update->alloc_ctx = &fiber()->gc; /* * Base field offset, e.g. 0 for C and 1 for Lua. Used only for * error messages. All fields numbers must be zero-based! @@ -1178,7 +1184,7 @@ const char * update_finish(struct tuple_update *update, uint32_t *p_tuple_len) { uint32_t tuple_len = update_calc_tuple_length(update); - char *buffer = (char *) update->alloc(update->alloc_ctx, tuple_len); + char *buffer = (char *) update_alloc(update->alloc_ctx, tuple_len); if (buffer == NULL) return NULL; *p_tuple_len = update_write_tuple(update, buffer, buffer + tuple_len); @@ -1186,23 +1192,21 @@ update_finish(struct tuple_update *update, uint32_t *p_tuple_len) } int -tuple_update_check_ops(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr, const char *expr_end, int index_base) +tuple_update_check_ops(const char *expr, const char *expr_end, int index_base) { struct tuple_update update; - update_init(&update, alloc, alloc_ctx, index_base); + update_init(&update, index_base); return update_read_ops(&update, expr, expr_end, 0); } const char * -tuple_update_execute(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr,const char *expr_end, +tuple_update_execute(const char *expr,const char *expr_end, const char *old_data, const char *old_data_end, uint32_t *p_tuple_len, int index_base, uint64_t *column_mask) { struct tuple_update update; - update_init(&update, alloc, alloc_ctx, index_base); + update_init(&update, index_base); uint32_t field_count = mp_decode_array(&old_data); if (update_read_ops(&update, expr, expr_end, field_count) != 0) @@ -1216,14 +1220,13 @@ tuple_update_execute(tuple_update_alloc_func alloc, void *alloc_ctx, } const char * -tuple_upsert_execute(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr,const char *expr_end, +tuple_upsert_execute(const char *expr,const char *expr_end, const char *old_data, const char *old_data_end, uint32_t *p_tuple_len, int index_base, bool suppress_error, uint64_t *column_mask) { struct tuple_update update; - update_init(&update, alloc, alloc_ctx, index_base); + update_init(&update, index_base); uint32_t field_count = mp_decode_array(&old_data); if (update_read_ops(&update, expr, expr_end, field_count) != 0) @@ -1238,8 +1241,7 @@ tuple_upsert_execute(tuple_update_alloc_func alloc, void *alloc_ctx, } const char * -tuple_upsert_squash(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr1, const char *expr1_end, +tuple_upsert_squash(const char *expr1, const char *expr1_end, const char *expr2, const char *expr2_end, size_t *result_size, int index_base) { @@ -1247,7 +1249,7 @@ tuple_upsert_squash(tuple_update_alloc_func alloc, void *alloc_ctx, const char *expr_end[2] = {expr1_end, expr2_end}; struct tuple_update update[2]; for (int j = 0; j < 2; j++) { - update_init(&update[j], alloc, alloc_ctx, index_base); + update_init(&update[j], index_base); if (update_read_ops(&update[j], expr[j], expr_end[j], 0)) return NULL; mp_decode_array(&expr[j]); @@ -1264,8 +1266,8 @@ tuple_upsert_squash(tuple_update_alloc_func alloc, void *alloc_ctx, } size_t possible_size = expr1_end - expr1 + expr2_end - expr2; const uint32_t space_for_arr_tag = 5; - char *buf = (char *)alloc(alloc_ctx, - possible_size + space_for_arr_tag); + char *buf = (char *) update_alloc(&fiber()->gc, + possible_size + space_for_arr_tag); if (buf == NULL) return NULL; /* reserve some space for mp array header */ diff --git a/src/box/tuple_update.h b/src/box/tuple_update.h index 706edd55a..37faa1918 100644 --- a/src/box/tuple_update.h +++ b/src/box/tuple_update.h @@ -44,22 +44,17 @@ enum { BOX_UPDATE_OP_CNT_MAX = 4000, }; -typedef void *(*tuple_update_alloc_func)(void *, size_t); - int -tuple_update_check_ops(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr, const char *expr_end, int index_base); +tuple_update_check_ops(const char *expr, const char *expr_end, int index_base); const char * -tuple_update_execute(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr,const char *expr_end, +tuple_update_execute(const char *expr,const char *expr_end, const char *old_data, const char *old_data_end, uint32_t *p_new_size, int index_base, uint64_t *column_mask); const char * -tuple_upsert_execute(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr, const char *expr_end, +tuple_upsert_execute(const char *expr, const char *expr_end, const char *old_data, const char *old_data_end, uint32_t *p_new_size, int index_base, bool suppress_error, uint64_t *column_mask); @@ -75,8 +70,7 @@ tuple_upsert_execute(tuple_update_alloc_func alloc, void *alloc_ctx, * If it isn't possible to merge expressions NULL is returned. */ const char * -tuple_upsert_squash(tuple_update_alloc_func alloc, void *alloc_ctx, - const char *expr1, const char *expr1_end, +tuple_upsert_squash(const char *expr1, const char *expr1_end, const char *expr2, const char *expr2_end, size_t *result_size, int index_base); diff --git a/src/box/vinyl.c b/src/box/vinyl.c index e0de65d05..d82cae766 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -1937,8 +1937,7 @@ vy_update(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt, uint32_t new_size, old_size; const char *old_tuple = tuple_data_range(stmt->old_tuple, &old_size); const char *old_tuple_end = old_tuple + old_size; - new_tuple = tuple_update_execute(region_aligned_alloc_cb, &fiber()->gc, - request->tuple, request->tuple_end, + new_tuple = tuple_update_execute(request->tuple, request->tuple_end, old_tuple, old_tuple_end, &new_size, request->index_base, &column_mask); if (new_tuple == NULL) @@ -2117,8 +2116,7 @@ vy_upsert(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt, if (vy_is_committed(env, pk)) return 0; /* Check update operations. */ - if (tuple_update_check_ops(region_aligned_alloc_cb, &fiber()->gc, - request->ops, request->ops_end, + if (tuple_update_check_ops(request->ops, request->ops_end, request->index_base)) { return -1; } @@ -2176,9 +2174,7 @@ vy_upsert(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt, old_tuple_end = old_tuple + old_size; /* Apply upsert operations to the old tuple. */ - new_tuple = tuple_upsert_execute(region_aligned_alloc_cb, - &fiber()->gc, ops, ops_end, - old_tuple, old_tuple_end, + new_tuple = tuple_upsert_execute(ops, ops_end, old_tuple, old_tuple_end, &new_size, 0, false, &column_mask); if (new_tuple == NULL) return -1; diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c index e5e53be79..1c55f86c2 100644 --- a/src/box/vy_upsert.c +++ b/src/box/vy_upsert.c @@ -38,17 +38,6 @@ #include "fiber.h" #include "column_mask.h" -static void * -vy_update_alloc(void *arg, size_t size) -{ - /* TODO: rewrite tuple_upsert_execute() without exceptions */ - struct region *region = (struct region *) arg; - void *data = region_aligned_alloc(region, size, sizeof(uint64_t)); - if (data == NULL) - diag_set(OutOfMemory, size, "region", "upsert"); - return data; -} - /** * Try to squash two upsert series (msgspacked index_base + ops) * Try to create a tuple with squahed operations @@ -58,7 +47,7 @@ vy_update_alloc(void *arg, size_t size) * @retval -1 - memory error */ static int -vy_upsert_try_to_squash(struct tuple_format *format, struct region *region, +vy_upsert_try_to_squash(struct tuple_format *format, const char *key_mp, const char *key_mp_end, const char *old_serie, const char *old_serie_end, const char *new_serie, const char *new_serie_end, @@ -68,8 +57,7 @@ vy_upsert_try_to_squash(struct tuple_format *format, struct region *region, size_t squashed_size; const char *squashed = - tuple_upsert_squash(vy_update_alloc, region, - old_serie, old_serie_end, + tuple_upsert_squash(old_serie, old_serie_end, new_serie, new_serie_end, &squashed_size, 0); if (squashed == NULL) @@ -130,10 +118,9 @@ vy_apply_upsert(struct tuple *new_stmt, struct tuple *old_stmt, size_t region_svp = region_used(region); uint8_t old_type = vy_stmt_type(old_stmt); uint64_t column_mask = COLUMN_MASK_FULL; - result_mp = tuple_upsert_execute(vy_update_alloc, region, new_ops, - new_ops_end, result_mp, result_mp_end, - &mp_size, 0, suppress_error, - &column_mask); + result_mp = tuple_upsert_execute(new_ops, new_ops_end, result_mp, + result_mp_end, &mp_size, 0, + suppress_error, &column_mask); result_mp_end = result_mp + mp_size; if (old_type != IPROTO_UPSERT) { assert(old_type == IPROTO_INSERT || @@ -163,10 +150,8 @@ vy_apply_upsert(struct tuple *new_stmt, struct tuple *old_stmt, * UPSERT + UPSERT case: combine operations */ assert(old_ops_end - old_ops > 0); - if (vy_upsert_try_to_squash(format, region, - result_mp, result_mp_end, - old_ops, old_ops_end, - new_ops, new_ops_end, + if (vy_upsert_try_to_squash(format, result_mp, result_mp_end, + old_ops, old_ops_end, new_ops, new_ops_end, &result_stmt) != 0) { region_truncate(region, region_svp); return NULL; diff --git a/test/unit/column_mask.c b/test/unit/column_mask.c index 7859626f8..3ffd351ac 100644 --- a/test/unit/column_mask.c +++ b/test/unit/column_mask.c @@ -3,6 +3,9 @@ #include "unit.h" #include "msgpuck.h" #include "trivia/util.h" +#include "fiber.h" +#include "memory.h" +#include "tuple.h" #define MAX_OPS 20 #define MAX_FIELDS 100 @@ -102,20 +105,6 @@ tuple_new_update(const struct tuple_update_template *update, char **end) return ret; } -static char buffer[2048]; -static size_t pos = 0; - -/** Allocator for the tuple_update function. */ -static void * -tuple_update_alloc_f(void *unused, size_t size) -{ - (void) unused; - fail_if(pos + size > sizeof(buffer)); - char *ret = &buffer[pos]; - pos += size; - return ret; -} - /** * Execute an update operation from the template and compare it * with the expected tuple and expected column_mask. @@ -138,20 +127,19 @@ check_update_result(const struct tuple_template *original, uint32_t actual_len; uint64_t column_mask; + struct region *region = &fiber()->gc; const char *actual = - tuple_update_execute(tuple_update_alloc_f, NULL, ops, ops_end, - old, old_end, &actual_len, 1, + tuple_update_execute(ops, ops_end, old, old_end, &actual_len, 1, &column_mask); fail_if(actual == NULL); is((int32_t)actual_len, new_end - new, "check result length"); is(memcmp(actual, new, actual_len), 0, "tuple update is correct"); is(column_mask, expected_mask, "column_mask is correct"); + fiber_gc(); free(old); free(new); free(ops); - /* reset the global buffer. */ - pos = 0; } static inline void @@ -239,6 +227,9 @@ basic_test() int main() { + memory_init(); + fiber_init(fiber_c_invoke); + tuple_init(NULL); header(); plan(27); @@ -246,4 +237,7 @@ main() footer(); check_plan(); + tuple_free(); + fiber_free(); + memory_free(); } -- 2.20.1 (Apple Git-117)
next prev parent reply other threads:[~2019-07-13 22:11 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-13 22:11 [PATCH 0/5] JSON update preparation Vladislav Shpilevoy 2019-07-13 22:11 ` Vladislav Shpilevoy [this message] 2019-07-13 22:11 ` [PATCH 2/5] rope: make rope library macro template Vladislav Shpilevoy 2019-07-13 22:11 ` [PATCH 3/5] tuple: relax struct tuple_update dependency on rope Vladislav Shpilevoy 2019-07-13 22:11 ` [PATCH 4/5] int96: add a missing header Vladislav Shpilevoy 2019-07-13 22:11 ` [PATCH 5/5] tuple: implement update by field name Vladislav Shpilevoy 2019-07-31 12:15 ` [PATCH 0/5] JSON update preparation Vladimir Davydov 2019-07-31 20:36 ` [tarantool-patches] " Vladislav Shpilevoy 2019-08-22 21:18 ` Vladislav Shpilevoy 2019-08-27 22:08 ` [tarantool-patches] " Kirill Yukhin
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=01397cda246da9b8dc54e7c5fbf92b38dfd1fdee.1563054879.git.v.shpilevoy@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH 1/5] tuple: remove alloc and alloc_ctx args from update()' \ /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