From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 30 Jul 2019 10:58:57 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH] txn: convert txn flags into bit mask Message-ID: <20190730075857.GF4659@esperanza> References: <7aafa5dcfee6521e4e73cc5f47640b7189b255e3.1564397497.git.vdavydov.dev@gmail.com> <20190729121410.GA16601@atlas> <20190729123002.GE4659@esperanza> <20190729153754.GA26958@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190729153754.GA26958@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Mon, Jul 29, 2019 at 06:37:54PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/07/29 15:33]: > > On Mon, Jul 29, 2019 at 03:14:11PM +0300, Konstantin Osipov wrote: > > > * Vladimir Davydov [19/07/29 13:56]: > > > > --- > > > > https://github.com/tarantool/tarantool/commits/dv/txn-flags > > > > > > Curious, why not use bit fields? > > I will read the article, I do not have a strong opinion, it's just > checking for bit flags using an enum looks a bit clumsy, how about > a bit of syntax sugar like > txn_has_flag(txn, FLAG), tnx_set_flag(txn, FLAG), > txn_clear_flag(txn, FLAG). > > What do you think? Sounds reasonable. Here you go: >From 16610dedb84c78a96d720d7bb1b1b881f0fa9d11 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 29 Jul 2019 13:49:04 +0300 Subject: [PATCH] txn: convert txn flags into bit mask diff --git a/src/box/txn.c b/src/box/txn.c index c7d01b2e..05ec6f14 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -197,10 +197,7 @@ txn_begin() txn->n_new_rows = 0; txn->n_local_rows = 0; txn->n_applier_rows = 0; - txn->has_triggers = false; - txn->is_done = false; - txn->can_yield = true; - txn->is_aborted_by_yield = false; + txn->flags = 0; txn->in_sub_stmt = 0; txn->id = ++tsn; txn->signature = -1; @@ -212,6 +209,12 @@ txn_begin() /* fiber_on_yield is initialized by engine on demand */ trigger_create(&txn->fiber_on_stop, txn_on_stop, NULL, NULL); trigger_add(&fiber()->on_stop, &txn->fiber_on_stop); + /* + * By default all transactions may yield. + * It's a responsibility of an engine to disable yields + * if they are not supported. + */ + txn_set_flag(txn, TXN_CAN_YIELD); return txn; } @@ -396,13 +399,13 @@ txn_complete(struct txn *txn) /* Undo the transaction. */ if (txn->engine) engine_rollback(txn->engine, txn); - if (txn->has_triggers) + if (txn_has_flag(txn, TXN_HAS_TRIGGERS)) txn_run_rollback_triggers(txn); } else { /* Commit the transaction. */ if (txn->engine != NULL) engine_commit(txn->engine, txn); - if (txn->has_triggers) + if (txn_has_flag(txn, TXN_HAS_TRIGGERS)) txn_run_commit_triggers(txn); double stop_tm = ev_monotonic_now(loop()); @@ -422,7 +425,7 @@ txn_complete(struct txn *txn) if (txn->fiber == NULL) txn_free(txn); else { - txn->is_done = true; + txn_set_flag(txn, TXN_IS_DONE); if (txn->fiber != fiber()) /* Wake a waiting fiber up. */ fiber_wakeup(txn->fiber); @@ -491,8 +494,8 @@ txn_write_to_wal(struct txn *txn) static int txn_prepare(struct txn *txn) { - if (txn->is_aborted_by_yield) { - assert(!txn->can_yield); + if (txn_has_flag(txn, TXN_IS_ABORTED_BY_YIELD)) { + assert(!txn_has_flag(txn, TXN_CAN_YIELD)); diag_set(ClientError, ER_TRANSACTION_YIELD); diag_log(); return -1; @@ -518,7 +521,7 @@ txn_prepare(struct txn *txn) return -1; } trigger_clear(&txn->fiber_on_stop); - if (!txn->can_yield) + if (!txn_has_flag(txn, TXN_CAN_YIELD)) trigger_clear(&txn->fiber_on_yield); return 0; } @@ -558,7 +561,7 @@ txn_commit(struct txn *txn) * In case of non-yielding journal the transaction could already * be done and there is nothing to wait in such cases. */ - if (!txn->is_done) { + if (!txn_has_flag(txn, TXN_IS_DONE)) { bool cancellable = fiber_set_cancellable(false); fiber_yield(); fiber_set_cancellable(cancellable); @@ -586,7 +589,7 @@ txn_rollback(struct txn *txn) { assert(txn == in_txn()); trigger_clear(&txn->fiber_on_stop); - if (!txn->can_yield) + if (!txn_has_flag(txn, TXN_CAN_YIELD)) trigger_clear(&txn->fiber_on_yield); txn->signature = -1; txn_complete(txn); @@ -607,11 +610,13 @@ void txn_can_yield(struct txn *txn, bool set) { assert(txn == in_txn()); - assert(txn->can_yield != set); - txn->can_yield = set; if (set) { + assert(!txn_has_flag(txn, TXN_CAN_YIELD)); + txn_set_flag(txn, TXN_CAN_YIELD); trigger_clear(&txn->fiber_on_yield); } else { + assert(txn_has_flag(txn, TXN_CAN_YIELD)); + txn_clear_flag(txn, TXN_CAN_YIELD); trigger_create(&txn->fiber_on_yield, txn_on_yield, NULL, NULL); trigger_add(&fiber()->on_yield, &txn->fiber_on_yield); } @@ -781,11 +786,12 @@ txn_on_yield(struct trigger *trigger, void *event) (void) trigger; (void) event; struct txn *txn = in_txn(); - assert(txn != NULL && !txn->can_yield); + assert(txn != NULL); + assert(!txn_has_flag(txn, TXN_CAN_YIELD)); txn_rollback_to_svp(txn, NULL); - if (txn->has_triggers) { + if (txn_has_flag(txn, TXN_HAS_TRIGGERS)) { txn_run_rollback_triggers(txn); - txn->has_triggers = false; + txn_clear_flag(txn, TXN_HAS_TRIGGERS); } - txn->is_aborted_by_yield = true; + txn_set_flag(txn, TXN_IS_ABORTED_BY_YIELD); } diff --git a/src/box/txn.h b/src/box/txn.h index 2d5dea21..0a5c5169 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -51,6 +51,23 @@ struct tuple; struct xrow_header; struct Vdbe; +enum txn_flag { + /** Transaction has been processed. */ + TXN_IS_DONE, + /** + * Transaction has been aborted by fiber yield so + * should be rolled back at commit. + */ + TXN_IS_ABORTED_BY_YIELD, + /** + * fiber_yield() is allowed inside the transaction. + * See txn_can_yield() for more details. + */ + TXN_CAN_YIELD, + /** on_commit and/or on_rollback list is not empty. */ + TXN_HAS_TRIGGERS, +}; + enum { /** * Maximum recursion depth for on_replace triggers. @@ -160,20 +177,8 @@ struct txn { * already assigned LSN. */ int n_applier_rows; - /* True when transaction is processed. */ - bool is_done; - /** - * True if the transaction was aborted so should be - * rolled back at commit. - */ - bool is_aborted_by_yield; - /** - * True if yields are allowed inside a transaction, - * see txn_can_yield(). - */ - bool can_yield; - /** True if on_commit and on_rollback lists are non-empty. */ - bool has_triggers; + /** Bit mask of transaction flags, see txn_flag. */ + unsigned flags; /** The number of active nested statement-level transactions. */ int8_t in_sub_stmt; /** @@ -206,6 +211,24 @@ struct txn { struct sql_txn *psql_txn; }; +static inline bool +txn_has_flag(struct txn *txn, enum txn_flag flag) +{ + return (txn->flags & (1 << flag)) != 0; +} + +static inline void +txn_set_flag(struct txn *txn, enum txn_flag flag) +{ + txn->flags |= 1 << flag; +} + +static inline void +txn_clear_flag(struct txn *txn, enum txn_flag flag) +{ + txn->flags &= ~(1 << flag); +} + /* Pointer to the current transaction (if any) */ static inline struct txn * in_txn() @@ -260,10 +283,10 @@ txn_write(struct txn *txn); static inline void txn_init_triggers(struct txn *txn) { - if (txn->has_triggers == false) { + if (!txn_has_flag(txn, TXN_HAS_TRIGGERS)) { rlist_create(&txn->on_commit); rlist_create(&txn->on_rollback); - txn->has_triggers = true; + txn_set_flag(txn, TXN_HAS_TRIGGERS); } }