Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions storage/innobase/include/dict0mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,10 @@ struct dict_table_t {
void lock_mutex_lock() { lock_latch.wr_lock(ut_d(SRW_LOCK_CALL)); }
/** Try to acquire exclusive lock_latch */
bool lock_mutex_trylock() { return lock_latch.wr_lock_try(); }
/** Try to acquire exclusive lock_latch with a bounded spin (no syscall,
no blocking), for the lock-release fast paths that hold trx->mutex.
@return whether lock_latch was acquired */
bool lock_mutex_trylock_spin() noexcept;
/** Release exclusive lock_latch */
void lock_mutex_unlock() { lock_latch.wr_unlock(); }
/** Acquire shared lock_latch */
Expand Down
4 changes: 4 additions & 0 deletions storage/innobase/include/lock0lock.h
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,10 @@ class lock_sys_t
/** @return whether this latch is possibly held by any thread */
bool is_locked() const noexcept { return lock.is_locked(); }
#endif
/** Try to acquire the latch with a bounded spin (no syscall, no
blocking), for the lock-release fast paths that hold trx->mutex.
@return whether the latch was acquired */
bool try_acquire_spin() noexcept;
};

public:
Expand Down
59 changes: 53 additions & 6 deletions storage/innobase/lock/lock0lock.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Created 5/7/1996 Heikki Tuuri
#include "que0que.h"
#include "scope.h"
#include "buf0buf.h"
#include "my_cpu.h"
#include <debug_sync.h>
#include <mysql/service_thd_mdl.h>

Expand Down Expand Up @@ -4529,6 +4530,52 @@ lock_rec_unlock(
g.cell(), first_lock, heap_no);
}

/** Spin budget for lock_release_try(), lock_release_on_prepare_try() and
lock_rec_unlock_unmodified() trylock attempts on per-cell and per-table
latches.

Those paths hold trx->mutex while attempting the trylock (the reverse of
the standard order used by lock_rec_convert_impl_to_expl()), so blocking
acquisition would risk deadlock. A single CAS attempt fails too readily
under contention: any one failure across the trx's record/table locks
marks the whole try-pass as unsuccessful, and after 5 such passes
lock_release() escalates to lock_sys.wr_lock() for the entire trx, which
then blocks every concurrent lock_sys.rd_lock() acquirer in lock_rec_lock()
and lock_table().

A bounded spin (CAS, PAUSE between attempts, no syscall, no blocking) lets a
transient holder release without changing the deadlock-avoidance guarantee. The
upper bound on extra trx->mutex hold time is LOCK_RELEASE_TRY_SPIN_BUDGET
* pause-cost (tens to ~100ns per iteration, microarchitecture-dependent). */
static constexpr unsigned LOCK_RELEASE_TRY_SPIN_BUDGET= 16;
Comment thread
iMineLink marked this conversation as resolved.
static_assert(LOCK_RELEASE_TRY_SPIN_BUDGET, "the !--spin loops would underflow");

ATTRIBUTE_NOINLINE
bool lock_sys_t::hash_latch::try_acquire_spin() noexcept
{
for (unsigned spin= LOCK_RELEASE_TRY_SPIN_BUDGET;;)
{
if (try_acquire())
return true;
if (!--spin)
return false;
MY_RELAX_CPU();
}
}

ATTRIBUTE_NOINLINE
bool dict_table_t::lock_mutex_trylock_spin() noexcept
{
for (unsigned spin= LOCK_RELEASE_TRY_SPIN_BUDGET;;)
{
if (lock_mutex_trylock())
return true;
if (!--spin)
return false;
MY_RELAX_CPU();
}
}

/** Release the explicit locks of a committing transaction,
and release possible other transactions waiting because of these locks.
@return whether the operation succeeded */
Expand Down Expand Up @@ -4573,7 +4620,7 @@ static bool lock_release_try(trx_t *trx)
auto &lock_hash= lock_sys.hash_get(lock->type_mode);
auto cell= lock_hash.cell_get(lock->un_member.rec_lock.page_id.fold());
auto latch= lock_sys_t::hash_table::latch(cell);
if (!latch->try_acquire())
if (!latch->try_acquire_spin())
all_released= false;
Comment thread
iMineLink marked this conversation as resolved.
Comment on lines 4622 to 4624

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the all_released be passed as a parameter? If we already lost one spin race, it might not make sense to spin on the other latches. Then again, our caller is going to invoke us up to 5 times and only after that escalate to acquiring exclusive lock_sys.latch. Maybe you already tested this and determined that attempting to spin on every latch leads to a better result?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop still drains the remaining locks even after all_released goes false, so at the next iteration (if it failed once), it'll have less work to do, more so if spinning is uniform and transients are mitigated for the remaining locks in a iteration. I did only test this variant (uninform spinning) and it worked fine, I did not test the other one, but I think I'll keep the uniform spinning here (at least in this first implementation).

else
{
Expand All @@ -4588,7 +4635,7 @@ static bool lock_release_try(trx_t *trx)
ut_ad(table->id >= DICT_HDR_FIRST_ID ||
(lock->mode() != LOCK_IX && lock->mode() != LOCK_X) ||
trx->dict_operation || trx->was_dict_operation);
if (!table->lock_mutex_trylock())
if (!table->lock_mutex_trylock_spin())
all_released= false;
else
{
Expand Down Expand Up @@ -4831,7 +4878,7 @@ bool lock_rec_unlock_unmodified(buf_block_t *block, hash_cell_t *cell,
computed cell address invalid */
cell= lock_sys.rec_hash.cell_get(
lock->un_member.rec_lock.page_id.fold());
if (!lock_sys_t::hash_table::latch(cell)->try_acquire())
if (!lock_sys_t::hash_table::latch(cell)->try_acquire_spin())
return false;
}
if (lock->trx != impl_trx)
Expand Down Expand Up @@ -4889,7 +4936,7 @@ static bool lock_release_on_prepare_try(trx_t *trx, bool unlock_unmodified)
const auto fold = lock->un_member.rec_lock.page_id.fold();
auto cell= lock_sys.rec_hash.cell_get(fold);
auto latch= lock_sys_t::hash_table::latch(cell);
if (latch->try_acquire())
if (latch->try_acquire_spin())
{
if (!rec_granted_exclusive_not_gap)
{
Expand Down Expand Up @@ -4926,7 +4973,7 @@ static bool lock_release_on_prepare_try(trx_t *trx, bool unlock_unmodified)
computed cell address invalid */
cell= lock_sys.rec_hash.cell_get(fold);
latch= lock_sys_t::hash_table::latch(cell);
if (latch->try_acquire())
if (latch->try_acquire_spin())
{
if (!lock_rec_unlock_unmodified<CELL>(block, cell, lock,
offsets))
Expand Down Expand Up @@ -4962,7 +5009,7 @@ static bool lock_release_on_prepare_try(trx_t *trx, bool unlock_unmodified)
switch (lock->mode()) {
case LOCK_IS:
case LOCK_S:
if (table->lock_mutex_trylock())
if (table->lock_mutex_trylock_spin())
{
lock_table_dequeue(lock, false);
table->lock_mutex_unlock();
Expand Down