MDEV-35701 trx_t::autoinc_locks causes unnecessary dynamic memory allocation #3720
Conversation
|
|
There was a problem hiding this comment.
Mainly we are replacing ib_vector_t* autoinc_locks; with small_vector. So, the possible impact mainly would depend on the stability of small_vector implementation. I think we need to test it well.
I added one comment on passing R value while adding the lock entry which doesn't seem completely right. I need a little more time to complete the review.
| ib_vector_push(trx->autoinc_locks, &lock); | ||
| trx->autoinc_locks.emplace_back(std::move(lock)); | ||
| goto allocated; |
There was a problem hiding this comment.
We are passing the pointer here as R value reference (ownership) and later using it again. It doesn't seem right.
allocated:
lock->type_mode = ib_uint32_t(type_mode | LOCK_TABLE);
lock->trx = trx;
Can we use L value reference instead ?
void emplace_back(T &arg)
trx->autoinc_locks.emplace_back(lock);
There was a problem hiding this comment.
I initially had lock there, but then assertions were failing because we failed to release auto-inc locks. However, &*lock does seem to work on those tests that failed for the very first revision.
…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. Reviewed by: Debarun Banerjee
Description
trx_t::autoinc_locks: Usesmall_vector<lock_t*,4>in order to avoid any dynamic memory allocation in the most common case (a statement is holdingAUTO_INCREMENTlocks on at most 4 tables or partitions).lock_cancel_waiting_and_release(): Instead of removing elements from the middle, simply assignnullptr, likelock_table_remove_autoinc_lock().Release Notes
This is a minor performance improvement.
How can this PR be tested?
A simple Sysbench is expected to show some impact. I did not test this yet.
Basing the PR against the correct MariaDB version
mainbranch.PR quality check