From 006ada1fed5c9c9ec510c05c63b75b02ccafddcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 20 Dec 2024 15:00:29 +0200 Subject: [PATCH] MDEV-35701 trx_t::autoinc_locks causes unnecessary dynamic memory allocation trx_t::autoinc_locks: Use small_vector in order to avoid any dynamic memory allocation in the most common case (a statement is holding AUTO_INCREMENT locks on at most 4 tables or partitions). lock_cancel_waiting_and_release(): Instead of removing elements from the middle, simply assign nullptr, like lock_table_remove_autoinc_lock(). --- storage/innobase/dict/drop.cc | 2 +- storage/innobase/include/small_vector.h | 10 ++ storage/innobase/include/trx0trx.h | 14 ++- storage/innobase/lock/lock0lock.cc | 137 +++++++++--------------- storage/innobase/trx/trx0trx.cc | 25 ++--- 5 files changed, 75 insertions(+), 113 deletions(-) diff --git a/storage/innobase/dict/drop.cc b/storage/innobase/dict/drop.cc index f46b4b04752d3..80bf8c97eadaa 100644 --- a/storage/innobase/dict/drop.cc +++ b/storage/innobase/dict/drop.cc @@ -247,7 +247,7 @@ void trx_t::commit(std::vector &deleted) mutex_lock(); lock_release_on_drop(this); ut_ad(UT_LIST_GET_LEN(lock.trx_locks) == 0); - ut_ad(ib_vector_is_empty(autoinc_locks)); + ut_ad(autoinc_locks.empty()); mem_heap_empty(lock.lock_heap); lock.table_locks.clear(); /* commit_persist() already reset this. */ diff --git a/storage/innobase/include/small_vector.h b/storage/innobase/include/small_vector.h index d28a36184b8e2..d6dfee3a438d6 100644 --- a/storage/innobase/include/small_vector.h +++ b/storage/innobase/include/small_vector.h @@ -67,6 +67,16 @@ class small_vector : public small_vector_base MEM_MAKE_ADDRESSABLE(small, sizeof small); } + void deep_clear() noexcept + { + if (small != BeginX) + { + my_free(BeginX); + BeginX= small; + } + set_size(0); + } + using iterator= T *; using const_iterator= const T *; using reverse_iterator= std::reverse_iterator; diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index e55c6f506ce18..9c31b1207ae01 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -32,10 +32,10 @@ Created 3/26/1996 Heikki Tuuri #include "que0types.h" #include "mem0mem.h" #include "trx0xa.h" -#include "ut0vec.h" #include "fts0fts.h" #include "read0types.h" #include "ilist.h" +#include "small_vector.h" #include @@ -864,12 +864,10 @@ struct trx_t : ilist_node<> ulint n_autoinc_rows; /*!< no. of AUTO-INC rows required for an SQL statement. This is useful for multi-row INSERTs */ - ib_vector_t* autoinc_locks; /* AUTOINC locks held by this - transaction. Note that these are - also in the lock list trx_locks. This - vector needs to be freed explicitly - when the trx instance is destroyed. - Protected by lock_sys.latch. */ + typedef small_vector autoinc_lock_vector; + /** AUTO_INCREMENT locks ehld by this transaction; a subset of trx_locks, + protected by lock_sys.latch. */ + autoinc_lock_vector autoinc_locks; /*------------------------------*/ bool read_only; /*!< true if transaction is flagged as a READ-ONLY transaction. @@ -1066,7 +1064,7 @@ struct trx_t : ilist_node<> ut_ad(!lock.wait_lock); ut_ad(UT_LIST_GET_LEN(lock.trx_locks) == 0); ut_ad(lock.table_locks.empty()); - ut_ad(!autoinc_locks || ib_vector_is_empty(autoinc_locks)); + ut_ad(autoinc_locks.empty()); ut_ad(UT_LIST_GET_LEN(lock.evicted_tables) == 0); ut_ad(!dict_operation); ut_ad(!apply_online_log); diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 1795852ec94d8..a921efd8236e5 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -2302,7 +2302,7 @@ static void lock_grant(lock_t *lock) dict_table_t *table= lock->un_member.tab_lock.table; ut_ad(!table->autoinc_trx); table->autoinc_trx= trx; - ib_vector_push(trx->autoinc_locks, &lock); + trx->autoinc_locks.emplace_back(std::move(lock)); } DBUG_PRINT("ib_lock", ("wait for trx " TRX_ID_FMT " ends", trx->id)); @@ -3646,7 +3646,7 @@ lock_t *lock_table_create(dict_table_t *table, unsigned type_mode, trx_t *trx, ut_ad(!table->autoinc_trx); table->autoinc_trx = trx; - ib_vector_push(trx->autoinc_locks, &lock); + trx->autoinc_locks.emplace_back(std::move(lock)); goto allocated; } @@ -3695,79 +3695,49 @@ lock_t *lock_table_create(dict_table_t *table, unsigned type_mode, trx_t *trx, return(lock); } -/*************************************************************//** -Pops autoinc lock requests from the transaction's autoinc_locks. We -handle the case where there are gaps in the array and they need to -be popped off the stack. */ -UNIV_INLINE -void -lock_table_pop_autoinc_locks( -/*=========================*/ - trx_t* trx) /*!< in/out: transaction that owns the AUTOINC locks */ -{ - ut_ad(!ib_vector_is_empty(trx->autoinc_locks)); - - /* Skip any gaps, gaps are NULL lock entries in the - trx->autoinc_locks vector. */ - - do { - ib_vector_pop(trx->autoinc_locks); - - if (ib_vector_is_empty(trx->autoinc_locks)) { - return; - } - - } while (*(lock_t**) ib_vector_get_last(trx->autoinc_locks) == NULL); -} - /*************************************************************//** Removes an autoinc lock request from the transaction's autoinc_locks. */ -UNIV_INLINE +static void lock_table_remove_autoinc_lock( /*===========================*/ lock_t* lock, /*!< in: table lock */ trx_t* trx) /*!< in/out: transaction that owns the lock */ { - ut_ad(lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)); - lock_sys.assert_locked(*lock->un_member.tab_lock.table); - ut_ad(trx->mutex_is_owner()); - - auto s = ib_vector_size(trx->autoinc_locks); - ut_ad(s); - - /* With stored functions and procedures the user may drop - a table within the same "statement". This special case has - to be handled by deleting only those AUTOINC locks that were - held by the table being dropped. */ - - lock_t* autoinc_lock = *static_cast( - ib_vector_get(trx->autoinc_locks, --s)); - - /* This is the default fast case. */ - - if (autoinc_lock == lock) { - lock_table_pop_autoinc_locks(trx); - } else { - /* The last element should never be NULL */ - ut_a(autoinc_lock != NULL); + ut_ad(lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)); + lock_sys.assert_locked(*lock->un_member.tab_lock.table); + ut_ad(trx->mutex_is_owner()); - /* Handle freeing the locks from within the stack. */ + auto begin= trx->autoinc_locks.begin(), end= trx->autoinc_locks.end(), i=end; + ut_ad(begin != end); - while (s) { - autoinc_lock = *static_cast( - ib_vector_get(trx->autoinc_locks, --s)); + /* With stored functions and procedures the user may drop a table + within the same "statement". This special case has to be handled by + deleting only those AUTOINC locks that were held by the table being + dropped. */ - if (autoinc_lock == lock) { - void* null_var = NULL; - ib_vector_set(trx->autoinc_locks, s, &null_var); - return; - } - } + if (*--i == lock) + { + /* This is the default fast case. */ + while (begin != i && !*--i) {} + trx->autoinc_locks.erase(i, end); + } + else + { + ut_a(*end); + /* Handle freeing the locks from within the stack. */ + while (begin != end) + { + if (*--end == lock) + { + *end= nullptr; + return; + } + } - /* Must find the autoinc lock. */ - ut_error; - } + /* Must find the autoinc lock. */ + ut_error; + } } /*************************************************************//** @@ -6443,44 +6413,31 @@ lock_clust_rec_read_check_and_lock_alt( return(err); } -/*******************************************************************//** -Check if a transaction holds any autoinc locks. -@return TRUE if the transaction holds any AUTOINC locks. */ -static -ibool -lock_trx_holds_autoinc_locks( -/*=========================*/ - const trx_t* trx) /*!< in: transaction */ -{ - ut_a(trx->autoinc_locks != NULL); - - return(!ib_vector_is_empty(trx->autoinc_locks)); -} - /** Release all AUTO_INCREMENT locks of the transaction. */ static void lock_release_autoinc_locks(trx_t *trx) { { + auto begin= trx->autoinc_locks.begin(), end= trx->autoinc_locks.end(); + ut_ad(begin != end); LockMutexGuard g{SRW_LOCK_CALL}; mysql_mutex_lock(&lock_sys.wait_mutex); trx->mutex_lock(); - auto autoinc_locks= trx->autoinc_locks; - ut_a(autoinc_locks); /* We release the locks in the reverse order. This is to avoid searching the vector for the element to delete at the lower level. See (lock_table_remove_low()) for details. */ - while (ulint size= ib_vector_size(autoinc_locks)) + do { - lock_t *lock= *static_cast - (ib_vector_get(autoinc_locks, size - 1)); + lock_t *lock= *--end; ut_ad(lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)); lock_table_dequeue(lock, true); lock_trx_table_locks_remove(lock); } + while (begin != end); } mysql_mutex_unlock(&lock_sys.wait_mutex); trx->mutex_unlock(); + trx->autoinc_locks.clear(); } /** Cancel a waiting lock request and release possibly waiting transactions */ @@ -6502,8 +6459,18 @@ void lock_cancel_waiting_and_release(lock_t *lock) { if (lock->type_mode == (LOCK_AUTO_INC | LOCK_TABLE)) { - ut_ad(trx->autoinc_locks); - ib_vector_remove(trx->autoinc_locks, lock); + /* This is similar to lock_table_remove_autoinc_lock() */ + auto begin= trx->autoinc_locks.begin(), end= trx->autoinc_locks.end(); + ut_ad(begin != end); + if (*--end == lock) + trx->autoinc_locks.erase(end, end + 1); + else + while (begin != end) + if (*--end == lock) + { + *end= nullptr; + break; + } } lock_table_dequeue(lock, true); /* Remove the lock from table lock vector too. */ @@ -6703,7 +6670,7 @@ lock_unlock_table_autoinc( ut_ad(trx_state == TRX_STATE_ACTIVE || trx_state == TRX_STATE_PREPARED || trx_state == TRX_STATE_NOT_STARTED); - if (lock_trx_holds_autoinc_locks(trx)) + if (!trx->autoinc_locks.empty()) lock_release_autoinc_locks(trx); } diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 94ed416eb60fe..a5919a747f76c 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -128,6 +128,8 @@ trx_init( trx->rsegs.m_noredo.rseg = NULL; + new(&trx->autoinc_locks) trx_t::autoinc_lock_vector(); + trx->read_only = false; trx->auto_commit = false; @@ -334,21 +336,12 @@ trx_t *trx_create() trx->assert_freed(); - mem_heap_t* heap; - ib_alloc_t* alloc; - /* We just got trx from pool, it should be non locking */ ut_ad(!trx->will_lock); ut_ad(!trx->rw_trx_hash_pins); DBUG_LOG("trx", "Create: " << trx); - heap = mem_heap_create(sizeof(ib_vector_t) + sizeof(void*) * 8); - - alloc = ib_heap_allocator_create(heap); - - trx->autoinc_locks = ib_vector_create(alloc, sizeof(void**), 4); - ut_ad(trx->mod_tables.empty()); ut_ad(trx->lock.n_rec_locks == 0); ut_ad(trx->lock.set_nth_bit_calls == 0); @@ -390,14 +383,8 @@ void trx_t::free() trx_sys.rw_trx_hash.put_pins(this); mysql_thd= nullptr; - // FIXME: We need to avoid this heap free/alloc for each commit. - if (autoinc_locks) - { - ut_ad(ib_vector_is_empty(autoinc_locks)); - /* We allocated a dedicated heap for the vector. */ - ib_vector_free(autoinc_locks); - autoinc_locks= NULL; - } + ut_ad(autoinc_locks.empty()); + autoinc_locks.deep_clear(); MEM_NOACCESS(&skip_lock_inheritance_and_n_ref, sizeof skip_lock_inheritance_and_n_ref); @@ -495,7 +482,7 @@ inline void trx_t::release_locks() lock_release(this); ut_ad(!lock.n_rec_locks); ut_ad(UT_LIST_GET_LEN(lock.trx_locks) == 0); - ut_ad(ib_vector_is_empty(autoinc_locks)); + ut_ad(autoinc_locks.empty()); mem_heap_empty(lock.lock_heap); } @@ -923,7 +910,7 @@ trx_start_low( trx->wsrep = wsrep_on(trx->mysql_thd); #endif /* WITH_WSREP */ - ut_a(ib_vector_is_empty(trx->autoinc_locks)); + ut_a(trx->autoinc_locks.empty()); ut_a(trx->lock.table_locks.empty()); /* No other thread can access this trx object through rw_trx_hash,