Skip to content

Commit

Permalink
MDEV-35701 trx_t::autoinc_locks causes unnecessary dynamic memory all…
Browse files Browse the repository at this point in the history 
…ocation

trx_t::autoinc_locks: Use small_vector<lock_t*,4> 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().

small_vector_base::Capacity: Remove. This only needs to be a template
parameter N (compile-time constant) in small_vector.
  • Loading branch information
@dr-m
dr-m committed Jan 2, 2025
1 parent 07b77e8 commit 1acd785
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 125 deletions.
2 changes: 1 addition & 1 deletion storage/innobase/dict/drop.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ void trx_t::commit(std::vector<pfs_os_file_t> &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. */
Expand Down
27 changes: 18 additions & 9 deletions storage/innobase/include/small_vector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,13 @@ class small_vector_base
protected:
typedef uint32_t Size_T;
void *BeginX;
Size_T Size= 0, Capacity;
Size_T Size= 0;
small_vector_base()= delete;
small_vector_base(void *small, size_t small_size)
: BeginX(small), Capacity(Size_T(small_size)) {}
ATTRIBUTE_COLD void grow_by_1(void *small, size_t element_size);
small_vector_base(void *small) : BeginX(small) {}
ATTRIBUTE_COLD
void grow_by_1(void *small, size_t element_size) noexcept;
public:
size_t size() const { return Size; }
size_t capacity() const { return Capacity; }
bool empty() const { return !Size; }
void clear() { Size= 0; }
protected:
Expand All @@ -49,24 +48,34 @@ class small_vector : public small_vector_base

using small_vector_base::set_size;

void grow_if_needed()
void grow_if_needed() noexcept
{
if (unlikely(size() >= capacity()))
if (unlikely(size() >= N))
grow_by_1(small, sizeof *small);
}

public:
small_vector() : small_vector_base(small, N)
small_vector() : small_vector_base(small)
{
TRASH_ALLOC(small, sizeof small);
}
~small_vector()
~small_vector() noexcept
{
if (small != begin())
my_free(begin());
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<iterator>;
Expand Down
14 changes: 6 additions & 8 deletions storage/innobase/include/trx0trx.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <vector>

Expand Down Expand Up @@ -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<lock_t*, 4> 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.
Expand Down Expand Up @@ -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);
Expand Down
137 changes: 52 additions & 85 deletions storage/innobase/lock/lock0lock.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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<lock_t**>(
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<lock_t**>(
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) i--;
trx->autoinc_locks.erase(i, end);
}
else
{
ut_a(*i);
/* Handle freeing the locks from within the stack. */
while (begin != i)
{
if (*--i == lock)
{
*i= nullptr;
return;
}
}

/* Must find the autoinc lock. */
ut_error;
}
/* Must find the autoinc lock. */
ut_error;
}
}

/*************************************************************//**
Expand Down Expand Up @@ -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<lock_t**>
(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 */
Expand All @@ -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. */
Expand Down Expand Up @@ -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);
}

Expand Down
6 changes: 3 additions & 3 deletions storage/innobase/mtr/mtr0mtr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1268,14 +1268,14 @@ void mtr_t::free(const fil_space_t &space, uint32_t offset)
m_log.close(log_write<FREE_PAGE>(id, nullptr));
}

void small_vector_base::grow_by_1(void *small, size_t element_size)
void small_vector_base::grow_by_1(void *small, size_t element_size) noexcept
{
const size_t cap= Capacity*= 2, s= cap * element_size;
const size_t s{2 * size() * element_size};
void *new_begin;
if (BeginX == small)
{
new_begin= my_malloc(PSI_NOT_INSTRUMENTED, s, MYF(0));
memcpy(new_begin, BeginX, size() * element_size);
memcpy(new_begin, BeginX, s / 2);
TRASH_FREE(small, size() * element_size);
}
else
Expand Down
25 changes: 6 additions & 19 deletions storage/innobase/trx/trx0trx.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 1acd785

Please sign in to comment.