Skip to content

Commit

Permalink
MDEV-28525 Some conditions around btr_latch_mode could be eliminated
Browse files Browse the repository at this point in the history 
The types btr_latch_mode and mtr_memo_type_t are partly derived from
rw_lock_type_t. Despite that, some code for converting between them
is using conditions instead of bitwise arithmetics.

Let us define btr_latch_mode in such a way that more conversions to
rw_lock_type_t are possible by bitwise and.

Some SPATIAL INDEX code that assumed !(BTR_MODIFY_TREE & BTR_MODIFY_LEAF)
was adjusted.
  • Loading branch information
@dr-m
dr-m committed Jun 6, 2022
1 parent aa45850 commit 75096c8
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 103 deletions.
47 changes: 25 additions & 22 deletions storage/innobase/btr/btr0cur.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,6 @@ btr_cur_latch_leaves(
btr_cur_t* cursor,
mtr_t* mtr)
{
rw_lock_type_t mode;
uint32_t left_page_no;
uint32_t right_page_no;
buf_block_t* get_block;
bool spatial;
btr_latch_leaves_t latch_leaves = {{NULL, NULL, NULL}, {0, 0, 0}};
Expand All @@ -230,7 +227,15 @@ btr_cur_latch_leaves(
| MTR_MEMO_X_LOCK
| MTR_MEMO_SX_LOCK));

const rw_lock_type_t mode = rw_lock_type_t(
latch_mode & (RW_X_LATCH | RW_S_LATCH));
static_assert(ulint{RW_S_LATCH} == ulint{BTR_SEARCH_LEAF}, "");
static_assert(ulint{RW_X_LATCH} == ulint{BTR_MODIFY_LEAF}, "");
static_assert(BTR_SEARCH_LEAF & BTR_SEARCH_TREE, "");

switch (latch_mode) {
uint32_t left_page_no;
uint32_t right_page_no;
case BTR_SEARCH_LEAF:
case BTR_MODIFY_LEAF:
case BTR_SEARCH_TREE:
Expand All @@ -239,7 +244,6 @@ btr_cur_latch_leaves(
= mtr_set_savepoint(mtr);
}

mode = latch_mode == BTR_MODIFY_LEAF ? RW_X_LATCH : RW_S_LATCH;
latch_leaves.savepoints[1] = mtr_set_savepoint(mtr);
get_block = btr_block_get(*cursor->index,
block->page.id().page_no(), mode,
Expand Down Expand Up @@ -335,7 +339,6 @@ btr_cur_latch_leaves(

case BTR_SEARCH_PREV:
case BTR_MODIFY_PREV:
mode = latch_mode == BTR_SEARCH_PREV ? RW_S_LATCH : RW_X_LATCH;
/* Because we are holding index->lock, no page splits
or merges may run concurrently, and we may read
FIL_PAGE_PREV from a buffer-fixed, unlatched page. */
Expand Down Expand Up @@ -787,8 +790,13 @@ btr_cur_optimistic_latch_leaves(
left_page_no = btr_page_get_prev(block->page.frame);
}

const rw_lock_type_t mode = *latch_mode == BTR_SEARCH_PREV
? RW_S_LATCH : RW_X_LATCH;
static_assert(BTR_SEARCH_PREV & BTR_SEARCH_LEAF, "");
static_assert(BTR_MODIFY_PREV & BTR_MODIFY_LEAF, "");
static_assert((BTR_SEARCH_PREV ^ BTR_MODIFY_PREV)
== (RW_S_LATCH ^ RW_X_LATCH), "");

const rw_lock_type_t mode = rw_lock_type_t(
*latch_mode & (RW_X_LATCH | RW_S_LATCH));

if (left_page_no != FIL_NULL) {
dberr_t err = DB_SUCCESS;
Expand Down Expand Up @@ -1328,10 +1336,8 @@ btr_cur_search_to_nth_level_func(
/* These flags are mutually exclusive, they are lumped together
with the latch mode for historical reasons. It's possible for
none of the flags to be set. */
switch (UNIV_EXPECT(latch_mode
& (BTR_INSERT | BTR_DELETE | BTR_DELETE_MARK),
0)) {
case 0:
switch (UNIV_EXPECT(latch_mode & BTR_DELETE, 0)) {
default:
btr_op = BTR_NO_OP;
break;
case BTR_INSERT:
Expand All @@ -1346,10 +1352,6 @@ btr_cur_search_to_nth_level_func(
case BTR_DELETE_MARK:
btr_op = BTR_DELMARK_OP;
break;
default:
/* only one of BTR_INSERT, BTR_DELETE, BTR_DELETE_MARK
should be specified at a time */
ut_error;
}

/* Operations on the insert buffer tree cannot be buffered. */
Expand Down Expand Up @@ -1941,17 +1943,18 @@ btr_cur_search_to_nth_level_func(
/* Need to use BTR_MODIFY_TREE to do the MBR adjustment */
if (search_mode == PAGE_CUR_RTREE_INSERT
&& cursor->rtr_info->mbr_adj) {
if (latch_mode & BTR_MODIFY_LEAF) {
static_assert(BTR_MODIFY_TREE
== (8 | BTR_MODIFY_LEAF), "");

if (!(latch_mode & 8)) {
/* Parent MBR needs updated, should retry
with BTR_MODIFY_TREE */
goto func_exit;
} else if (latch_mode & BTR_MODIFY_TREE) {
rtree_parent_modified = true;
cursor->rtr_info->mbr_adj = false;
mbr_adj = true;
} else {
ut_ad(0);
}

rtree_parent_modified = true;
cursor->rtr_info->mbr_adj = false;
mbr_adj = true;
}

if (found && page_mode == PAGE_CUR_RTREE_GET_FATHER) {
Expand Down
25 changes: 7 additions & 18 deletions storage/innobase/btr/btr0pcur.cc
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2016, 2021, MariaDB Corporation.
Copyright (c) 2016, 2022, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -547,34 +547,23 @@ btr_pcur_move_backward_from_page(
ulint prev_page_no;
page_t* page;
buf_block_t* prev_block;
ulint latch_mode;
ulint latch_mode2;

ut_ad(cursor->latch_mode != BTR_NO_LATCHES);
ut_ad(btr_pcur_is_before_first_on_page(cursor));
ut_ad(!btr_pcur_is_before_first_in_tree(cursor));

latch_mode = cursor->latch_mode;

if (latch_mode == BTR_SEARCH_LEAF) {

latch_mode2 = BTR_SEARCH_PREV;

} else if (latch_mode == BTR_MODIFY_LEAF) {

latch_mode2 = BTR_MODIFY_PREV;
} else {
latch_mode2 = 0; /* To eliminate compiler warning */
ut_error;
}
const ulint latch_mode = cursor->latch_mode;
ut_ad(latch_mode == BTR_SEARCH_LEAF || latch_mode == BTR_MODIFY_LEAF);

btr_pcur_store_position(cursor, mtr);

mtr_commit(mtr);

mtr_start(mtr);

cursor->restore_position(latch_mode2, mtr);
static_assert(BTR_SEARCH_PREV == (4 | BTR_SEARCH_LEAF), "");
static_assert(BTR_MODIFY_PREV == (4 | BTR_MODIFY_LEAF), "");

cursor->restore_position(4 | latch_mode, mtr);

page = btr_pcur_get_page(cursor);

Expand Down
17 changes: 8 additions & 9 deletions storage/innobase/btr/btr0sea.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1124,15 +1124,14 @@ btr_search_guess_on_hash(
block->page.fix();
block->page.set_accessed();
buf_page_make_young_if_needed(&block->page);
mtr_memo_type_t fix_type;
if (latch_mode == BTR_SEARCH_LEAF) {
fix_type = MTR_MEMO_PAGE_S_FIX;
ut_ad(!block->page.is_read_fixed());
} else {
fix_type = MTR_MEMO_PAGE_X_FIX;
ut_ad(!block->page.is_io_fixed());
}
mtr->memo_push(block, fix_type);
ut_ad(!block->page.is_read_fixed());
ut_ad(latch_mode == BTR_SEARCH_LEAF
|| !block->page.is_io_fixed());
static_assert(ulint{MTR_MEMO_PAGE_S_FIX} ==
ulint{BTR_SEARCH_LEAF}, "");
static_assert(ulint{MTR_MEMO_PAGE_X_FIX} ==
ulint{BTR_MODIFY_LEAF}, "");
mtr->memo_push(block, mtr_memo_type_t(latch_mode));

++buf_pool.stat.n_page_gets;

Expand Down
46 changes: 20 additions & 26 deletions storage/innobase/gis/gis0sea.cc
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2016, 2018, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2021, MariaDB Corporation.
Copyright (c) 2017, 2022, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -105,7 +105,6 @@ rtr_pcur_getnext_from_path(
ulint my_latch_mode;
ulint skip_parent = false;
bool new_split = false;
bool need_parent;
bool for_delete = false;
bool for_undo_ins = false;

Expand All @@ -131,13 +130,12 @@ rtr_pcur_getnext_from_path(

/* Whether need to track parent information. Only need so
when we do tree altering operations (such as index page merge) */
need_parent = ((my_latch_mode == BTR_MODIFY_TREE
|| my_latch_mode == BTR_CONT_MODIFY_TREE)
&& mode == PAGE_CUR_RTREE_LOCATE);
static_assert(BTR_CONT_MODIFY_TREE == (4 | BTR_MODIFY_TREE), "");

const bool need_parent = mode == PAGE_CUR_RTREE_LOCATE
&& (my_latch_mode | 4) == BTR_CONT_MODIFY_TREE;

if (!index_locked) {
ut_ad(latch_mode & BTR_SEARCH_LEAF
|| latch_mode & BTR_MODIFY_LEAF);
mtr_s_lock_index(index, mtr);
} else {
ut_ad(mtr->memo_contains_flagged(&index->lock,
Expand All @@ -156,7 +154,7 @@ rtr_pcur_getnext_from_path(
buf_block_t* block;
node_seq_t path_ssn;
const page_t* page;
ulint rw_latch = RW_X_LATCH;
rw_lock_type_t rw_latch;
ulint tree_idx;

mysql_mutex_lock(&rtr_info->rtr_path_mutex);
Expand Down Expand Up @@ -215,18 +213,14 @@ rtr_pcur_getnext_from_path(
One reason for pre-latch is that we might need to position
some parent position (requires latch) during search */
if (level == 0) {
/* S latched for SEARCH_LEAF, and X latched
for MODIFY_LEAF */
if (my_latch_mode <= BTR_MODIFY_LEAF) {
rw_latch = my_latch_mode;
}

if (my_latch_mode == BTR_CONT_MODIFY_TREE
|| my_latch_mode == BTR_MODIFY_TREE) {
rw_latch = RW_NO_LATCH;
}

} else if (level == target_level) {
static_assert(ulint{BTR_SEARCH_LEAF} ==
ulint{RW_S_LATCH}, "");
static_assert(ulint{BTR_MODIFY_LEAF} ==
ulint{RW_X_LATCH}, "");
rw_latch = (my_latch_mode | 4) == BTR_CONT_MODIFY_TREE
? RW_NO_LATCH
: rw_lock_type_t(my_latch_mode);
} else {
rw_latch = RW_X_LATCH;
}

Expand Down Expand Up @@ -257,8 +251,7 @@ rtr_pcur_getnext_from_path(
/* set up savepoint to record any locks to be taken */
rtr_info->tree_savepoints[tree_idx] = mtr_set_savepoint(mtr);

ut_ad(my_latch_mode == BTR_MODIFY_TREE
|| my_latch_mode == BTR_CONT_MODIFY_TREE
ut_ad((my_latch_mode | 4) == BTR_CONT_MODIFY_TREE
|| !page_is_leaf(btr_cur_get_page(btr_cur))
|| !btr_cur->page_cur.block->page.lock.have_any());

Expand Down Expand Up @@ -543,7 +536,8 @@ rtr_pcur_open(
ulint low_match;
rec_t* rec;

ut_ad(latch_mode & BTR_MODIFY_LEAF || latch_mode & BTR_MODIFY_TREE);
static_assert(BTR_MODIFY_TREE == (8 | BTR_MODIFY_LEAF), "");
ut_ad(latch_mode & BTR_MODIFY_LEAF);
ut_ad(mode == PAGE_CUR_RTREE_LOCATE);

/* Initialize the cursor */
Expand All @@ -566,7 +560,7 @@ rtr_pcur_open(
btr_cursor->rtr_info->thr = btr_cursor->thr;
}

if ((latch_mode & BTR_MODIFY_TREE) && index->lock.have_u_not_x()) {
if ((latch_mode & 8) && index->lock.have_u_not_x()) {
index->lock.u_x_upgrade(SRW_LOCK_CALL);
mtr->lock_upgrade(index->lock);
}
Expand Down Expand Up @@ -595,7 +589,7 @@ rtr_pcur_open(
}
/* Did not find matched row in first dive. Release
latched block if any before search more pages */
if (latch_mode & BTR_MODIFY_LEAF) {
if (!(latch_mode & 8)) {
ulint tree_idx = btr_cursor->tree_height - 1;
rtr_info_t* rtr_info = btr_cursor->rtr_info;

Expand All @@ -610,7 +604,7 @@ rtr_pcur_open(

bool ret = rtr_pcur_getnext_from_path(
tuple, mode, btr_cursor, 0, latch_mode,
latch_mode & (BTR_MODIFY_TREE | BTR_ALREADY_S_LATCHED),
latch_mode & (8 | BTR_ALREADY_S_LATCHED),
mtr);

if (ret) {
Expand Down
34 changes: 17 additions & 17 deletions storage/innobase/include/btr0btr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,37 +63,37 @@ enum btr_latch_mode {
BTR_MODIFY_LEAF = RW_X_LATCH,
/** Obtain no latches. */
BTR_NO_LATCHES = RW_NO_LATCH,
/** Start modifying the entire B-tree. */
BTR_MODIFY_TREE = 33,
/** Continue modifying the entire B-tree. */
BTR_CONT_MODIFY_TREE = 34,
/** Search the previous record. */
BTR_SEARCH_PREV = 35,
BTR_SEARCH_PREV = 4 | BTR_SEARCH_LEAF,
/** Modify the previous record. */
BTR_MODIFY_PREV = 36,
BTR_MODIFY_PREV = 4 | BTR_MODIFY_LEAF,
/** Start searching the entire B-tree. */
BTR_SEARCH_TREE = 37,
BTR_SEARCH_TREE = 8 | BTR_SEARCH_LEAF,
/** Start modifying1 the entire B-tree. */
BTR_MODIFY_TREE = 8 | BTR_MODIFY_LEAF,
/** Continue searching the entire B-tree. */
BTR_CONT_SEARCH_TREE = 38,
BTR_CONT_SEARCH_TREE = 4 | BTR_SEARCH_TREE,
/** Continue modifying the entire B-tree. */
BTR_CONT_MODIFY_TREE = 4 | BTR_MODIFY_TREE,

/* BTR_INSERT, BTR_DELETE and BTR_DELETE_MARK are mutually
exclusive. */
/** The search tuple will be inserted to the secondary index
at the searched position. When the leaf page is not in the
buffer pool, try to use the change buffer. */
BTR_INSERT = 512,
BTR_INSERT = 64,

/** Try to delete mark a secondary index leaf page record at
the searched position using the change buffer when the page is
not in the buffer pool. */
BTR_DELETE_MARK = 4096,
BTR_DELETE_MARK = 128,

/** Try to purge the record using the change buffer when the
secondary index leaf page is not in the buffer pool. */
BTR_DELETE = 8192,
BTR_DELETE = BTR_INSERT | BTR_DELETE_MARK,

/** The caller is already holding dict_index_t::lock S-latch. */
BTR_ALREADY_S_LATCHED = 16384,
BTR_ALREADY_S_LATCHED = 256,
/** Search and S-latch a leaf page, assuming that the
dict_index_t::lock S-latch is being held. */
BTR_SEARCH_LEAF_ALREADY_S_LATCHED = BTR_SEARCH_LEAF
Expand Down Expand Up @@ -123,7 +123,7 @@ enum btr_latch_mode {
/** In the case of BTR_MODIFY_TREE, the caller specifies
the intention to delete record only. It is used to optimize
block->lock range.*/
BTR_LATCH_FOR_DELETE = 65536,
BTR_LATCH_FOR_DELETE = 512,

/** Attempt to purge a secondary index record in the tree. */
BTR_PURGE_TREE = BTR_MODIFY_TREE | BTR_LATCH_FOR_DELETE
Expand All @@ -140,19 +140,19 @@ the insert buffer to speed up inserts */

/** In the case of BTR_MODIFY_TREE, the caller specifies the intention
to insert record only. It is used to optimize block->lock range.*/
#define BTR_LATCH_FOR_INSERT 32768U
#define BTR_LATCH_FOR_INSERT 4096U

/** This flag is for undo insert of rtree. For rtree, we need this flag
to find proper rec to undo insert.*/
#define BTR_RTREE_UNDO_INS 131072U
#define BTR_RTREE_UNDO_INS 8192U

/** In the case of BTR_MODIFY_LEAF, the caller intends to allocate or
free the pages of externally stored fields. */
#define BTR_MODIFY_EXTERNAL 262144U
#define BTR_MODIFY_EXTERNAL 16384U

/** Try to delete mark the record at the searched position when the
record is in spatial index */
#define BTR_RTREE_DELETE_MARK 524288U
#define BTR_RTREE_DELETE_MARK 32768U

#define BTR_LATCH_MODE_WITHOUT_FLAGS(latch_mode) \
((latch_mode) & ulint(~(BTR_INSERT \
Expand Down
Loading

0 comments on commit 75096c8

Please sign in to comment.