Skip to content

Commit 75096c8

Browse files
committed
MDEV-28525 Some conditions around btr_latch_mode could be eliminated
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.
1 parent aa45850 commit 75096c8

File tree

8 files changed

+87
-103
lines changed

8 files changed

+87
-103
lines changed

storage/innobase/btr/btr0cur.cc

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,6 @@ btr_cur_latch_leaves(
210210
btr_cur_t* cursor,
211211
mtr_t* mtr)
212212
{
213-
rw_lock_type_t mode;
214-
uint32_t left_page_no;
215-
uint32_t right_page_no;
216213
buf_block_t* get_block;
217214
bool spatial;
218215
btr_latch_leaves_t latch_leaves = {{NULL, NULL, NULL}, {0, 0, 0}};
@@ -230,7 +227,15 @@ btr_cur_latch_leaves(
230227
| MTR_MEMO_X_LOCK
231228
| MTR_MEMO_SX_LOCK));
232229

230+
const rw_lock_type_t mode = rw_lock_type_t(
231+
latch_mode & (RW_X_LATCH | RW_S_LATCH));
232+
static_assert(ulint{RW_S_LATCH} == ulint{BTR_SEARCH_LEAF}, "");
233+
static_assert(ulint{RW_X_LATCH} == ulint{BTR_MODIFY_LEAF}, "");
234+
static_assert(BTR_SEARCH_LEAF & BTR_SEARCH_TREE, "");
235+
233236
switch (latch_mode) {
237+
uint32_t left_page_no;
238+
uint32_t right_page_no;
234239
case BTR_SEARCH_LEAF:
235240
case BTR_MODIFY_LEAF:
236241
case BTR_SEARCH_TREE:
@@ -239,7 +244,6 @@ btr_cur_latch_leaves(
239244
= mtr_set_savepoint(mtr);
240245
}
241246

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

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

790-
const rw_lock_type_t mode = *latch_mode == BTR_SEARCH_PREV
791-
? RW_S_LATCH : RW_X_LATCH;
793+
static_assert(BTR_SEARCH_PREV & BTR_SEARCH_LEAF, "");
794+
static_assert(BTR_MODIFY_PREV & BTR_MODIFY_LEAF, "");
795+
static_assert((BTR_SEARCH_PREV ^ BTR_MODIFY_PREV)
796+
== (RW_S_LATCH ^ RW_X_LATCH), "");
797+
798+
const rw_lock_type_t mode = rw_lock_type_t(
799+
*latch_mode & (RW_X_LATCH | RW_S_LATCH));
792800

793801
if (left_page_no != FIL_NULL) {
794802
dberr_t err = DB_SUCCESS;
@@ -1328,10 +1336,8 @@ btr_cur_search_to_nth_level_func(
13281336
/* These flags are mutually exclusive, they are lumped together
13291337
with the latch mode for historical reasons. It's possible for
13301338
none of the flags to be set. */
1331-
switch (UNIV_EXPECT(latch_mode
1332-
& (BTR_INSERT | BTR_DELETE | BTR_DELETE_MARK),
1333-
0)) {
1334-
case 0:
1339+
switch (UNIV_EXPECT(latch_mode & BTR_DELETE, 0)) {
1340+
default:
13351341
btr_op = BTR_NO_OP;
13361342
break;
13371343
case BTR_INSERT:
@@ -1346,10 +1352,6 @@ btr_cur_search_to_nth_level_func(
13461352
case BTR_DELETE_MARK:
13471353
btr_op = BTR_DELMARK_OP;
13481354
break;
1349-
default:
1350-
/* only one of BTR_INSERT, BTR_DELETE, BTR_DELETE_MARK
1351-
should be specified at a time */
1352-
ut_error;
13531355
}
13541356

13551357
/* Operations on the insert buffer tree cannot be buffered. */
@@ -1941,17 +1943,18 @@ btr_cur_search_to_nth_level_func(
19411943
/* Need to use BTR_MODIFY_TREE to do the MBR adjustment */
19421944
if (search_mode == PAGE_CUR_RTREE_INSERT
19431945
&& cursor->rtr_info->mbr_adj) {
1944-
if (latch_mode & BTR_MODIFY_LEAF) {
1946+
static_assert(BTR_MODIFY_TREE
1947+
== (8 | BTR_MODIFY_LEAF), "");
1948+
1949+
if (!(latch_mode & 8)) {
19451950
/* Parent MBR needs updated, should retry
19461951
with BTR_MODIFY_TREE */
19471952
goto func_exit;
1948-
} else if (latch_mode & BTR_MODIFY_TREE) {
1949-
rtree_parent_modified = true;
1950-
cursor->rtr_info->mbr_adj = false;
1951-
mbr_adj = true;
1952-
} else {
1953-
ut_ad(0);
19541953
}
1954+
1955+
rtree_parent_modified = true;
1956+
cursor->rtr_info->mbr_adj = false;
1957+
mbr_adj = true;
19551958
}
19561959

19571960
if (found && page_mode == PAGE_CUR_RTREE_GET_FATHER) {

storage/innobase/btr/btr0pcur.cc

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*****************************************************************************
22
33
Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
4-
Copyright (c) 2016, 2021, MariaDB Corporation.
4+
Copyright (c) 2016, 2022, MariaDB Corporation.
55
66
This program is free software; you can redistribute it and/or modify it under
77
the terms of the GNU General Public License as published by the Free Software
@@ -547,34 +547,23 @@ btr_pcur_move_backward_from_page(
547547
ulint prev_page_no;
548548
page_t* page;
549549
buf_block_t* prev_block;
550-
ulint latch_mode;
551-
ulint latch_mode2;
552550

553-
ut_ad(cursor->latch_mode != BTR_NO_LATCHES);
554551
ut_ad(btr_pcur_is_before_first_on_page(cursor));
555552
ut_ad(!btr_pcur_is_before_first_in_tree(cursor));
556553

557-
latch_mode = cursor->latch_mode;
558-
559-
if (latch_mode == BTR_SEARCH_LEAF) {
560-
561-
latch_mode2 = BTR_SEARCH_PREV;
562-
563-
} else if (latch_mode == BTR_MODIFY_LEAF) {
564-
565-
latch_mode2 = BTR_MODIFY_PREV;
566-
} else {
567-
latch_mode2 = 0; /* To eliminate compiler warning */
568-
ut_error;
569-
}
554+
const ulint latch_mode = cursor->latch_mode;
555+
ut_ad(latch_mode == BTR_SEARCH_LEAF || latch_mode == BTR_MODIFY_LEAF);
570556

571557
btr_pcur_store_position(cursor, mtr);
572558

573559
mtr_commit(mtr);
574560

575561
mtr_start(mtr);
576562

577-
cursor->restore_position(latch_mode2, mtr);
563+
static_assert(BTR_SEARCH_PREV == (4 | BTR_SEARCH_LEAF), "");
564+
static_assert(BTR_MODIFY_PREV == (4 | BTR_MODIFY_LEAF), "");
565+
566+
cursor->restore_position(4 | latch_mode, mtr);
578567

579568
page = btr_pcur_get_page(cursor);
580569

storage/innobase/btr/btr0sea.cc

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,15 +1124,14 @@ btr_search_guess_on_hash(
11241124
block->page.fix();
11251125
block->page.set_accessed();
11261126
buf_page_make_young_if_needed(&block->page);
1127-
mtr_memo_type_t fix_type;
1128-
if (latch_mode == BTR_SEARCH_LEAF) {
1129-
fix_type = MTR_MEMO_PAGE_S_FIX;
1130-
ut_ad(!block->page.is_read_fixed());
1131-
} else {
1132-
fix_type = MTR_MEMO_PAGE_X_FIX;
1133-
ut_ad(!block->page.is_io_fixed());
1134-
}
1135-
mtr->memo_push(block, fix_type);
1127+
ut_ad(!block->page.is_read_fixed());
1128+
ut_ad(latch_mode == BTR_SEARCH_LEAF
1129+
|| !block->page.is_io_fixed());
1130+
static_assert(ulint{MTR_MEMO_PAGE_S_FIX} ==
1131+
ulint{BTR_SEARCH_LEAF}, "");
1132+
static_assert(ulint{MTR_MEMO_PAGE_X_FIX} ==
1133+
ulint{BTR_MODIFY_LEAF}, "");
1134+
mtr->memo_push(block, mtr_memo_type_t(latch_mode));
11361135

11371136
++buf_pool.stat.n_page_gets;
11381137

storage/innobase/gis/gis0sea.cc

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*****************************************************************************
22
33
Copyright (c) 2016, 2018, Oracle and/or its affiliates. All Rights Reserved.
4-
Copyright (c) 2017, 2021, MariaDB Corporation.
4+
Copyright (c) 2017, 2022, MariaDB Corporation.
55
66
This program is free software; you can redistribute it and/or modify it under
77
the terms of the GNU General Public License as published by the Free Software
@@ -105,7 +105,6 @@ rtr_pcur_getnext_from_path(
105105
ulint my_latch_mode;
106106
ulint skip_parent = false;
107107
bool new_split = false;
108-
bool need_parent;
109108
bool for_delete = false;
110109
bool for_undo_ins = false;
111110

@@ -131,13 +130,12 @@ rtr_pcur_getnext_from_path(
131130

132131
/* Whether need to track parent information. Only need so
133132
when we do tree altering operations (such as index page merge) */
134-
need_parent = ((my_latch_mode == BTR_MODIFY_TREE
135-
|| my_latch_mode == BTR_CONT_MODIFY_TREE)
136-
&& mode == PAGE_CUR_RTREE_LOCATE);
133+
static_assert(BTR_CONT_MODIFY_TREE == (4 | BTR_MODIFY_TREE), "");
134+
135+
const bool need_parent = mode == PAGE_CUR_RTREE_LOCATE
136+
&& (my_latch_mode | 4) == BTR_CONT_MODIFY_TREE;
137137

138138
if (!index_locked) {
139-
ut_ad(latch_mode & BTR_SEARCH_LEAF
140-
|| latch_mode & BTR_MODIFY_LEAF);
141139
mtr_s_lock_index(index, mtr);
142140
} else {
143141
ut_ad(mtr->memo_contains_flagged(&index->lock,
@@ -156,7 +154,7 @@ rtr_pcur_getnext_from_path(
156154
buf_block_t* block;
157155
node_seq_t path_ssn;
158156
const page_t* page;
159-
ulint rw_latch = RW_X_LATCH;
157+
rw_lock_type_t rw_latch;
160158
ulint tree_idx;
161159

162160
mysql_mutex_lock(&rtr_info->rtr_path_mutex);
@@ -215,18 +213,14 @@ rtr_pcur_getnext_from_path(
215213
One reason for pre-latch is that we might need to position
216214
some parent position (requires latch) during search */
217215
if (level == 0) {
218-
/* S latched for SEARCH_LEAF, and X latched
219-
for MODIFY_LEAF */
220-
if (my_latch_mode <= BTR_MODIFY_LEAF) {
221-
rw_latch = my_latch_mode;
222-
}
223-
224-
if (my_latch_mode == BTR_CONT_MODIFY_TREE
225-
|| my_latch_mode == BTR_MODIFY_TREE) {
226-
rw_latch = RW_NO_LATCH;
227-
}
228-
229-
} else if (level == target_level) {
216+
static_assert(ulint{BTR_SEARCH_LEAF} ==
217+
ulint{RW_S_LATCH}, "");
218+
static_assert(ulint{BTR_MODIFY_LEAF} ==
219+
ulint{RW_X_LATCH}, "");
220+
rw_latch = (my_latch_mode | 4) == BTR_CONT_MODIFY_TREE
221+
? RW_NO_LATCH
222+
: rw_lock_type_t(my_latch_mode);
223+
} else {
230224
rw_latch = RW_X_LATCH;
231225
}
232226

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

260-
ut_ad(my_latch_mode == BTR_MODIFY_TREE
261-
|| my_latch_mode == BTR_CONT_MODIFY_TREE
254+
ut_ad((my_latch_mode | 4) == BTR_CONT_MODIFY_TREE
262255
|| !page_is_leaf(btr_cur_get_page(btr_cur))
263256
|| !btr_cur->page_cur.block->page.lock.have_any());
264257

@@ -543,7 +536,8 @@ rtr_pcur_open(
543536
ulint low_match;
544537
rec_t* rec;
545538

546-
ut_ad(latch_mode & BTR_MODIFY_LEAF || latch_mode & BTR_MODIFY_TREE);
539+
static_assert(BTR_MODIFY_TREE == (8 | BTR_MODIFY_LEAF), "");
540+
ut_ad(latch_mode & BTR_MODIFY_LEAF);
547541
ut_ad(mode == PAGE_CUR_RTREE_LOCATE);
548542

549543
/* Initialize the cursor */
@@ -566,7 +560,7 @@ rtr_pcur_open(
566560
btr_cursor->rtr_info->thr = btr_cursor->thr;
567561
}
568562

569-
if ((latch_mode & BTR_MODIFY_TREE) && index->lock.have_u_not_x()) {
563+
if ((latch_mode & 8) && index->lock.have_u_not_x()) {
570564
index->lock.u_x_upgrade(SRW_LOCK_CALL);
571565
mtr->lock_upgrade(index->lock);
572566
}
@@ -595,7 +589,7 @@ rtr_pcur_open(
595589
}
596590
/* Did not find matched row in first dive. Release
597591
latched block if any before search more pages */
598-
if (latch_mode & BTR_MODIFY_LEAF) {
592+
if (!(latch_mode & 8)) {
599593
ulint tree_idx = btr_cursor->tree_height - 1;
600594
rtr_info_t* rtr_info = btr_cursor->rtr_info;
601595

@@ -610,7 +604,7 @@ rtr_pcur_open(
610604

611605
bool ret = rtr_pcur_getnext_from_path(
612606
tuple, mode, btr_cursor, 0, latch_mode,
613-
latch_mode & (BTR_MODIFY_TREE | BTR_ALREADY_S_LATCHED),
607+
latch_mode & (8 | BTR_ALREADY_S_LATCHED),
614608
mtr);
615609

616610
if (ret) {

storage/innobase/include/btr0btr.h

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,37 +63,37 @@ enum btr_latch_mode {
6363
BTR_MODIFY_LEAF = RW_X_LATCH,
6464
/** Obtain no latches. */
6565
BTR_NO_LATCHES = RW_NO_LATCH,
66-
/** Start modifying the entire B-tree. */
67-
BTR_MODIFY_TREE = 33,
68-
/** Continue modifying the entire B-tree. */
69-
BTR_CONT_MODIFY_TREE = 34,
7066
/** Search the previous record. */
71-
BTR_SEARCH_PREV = 35,
67+
BTR_SEARCH_PREV = 4 | BTR_SEARCH_LEAF,
7268
/** Modify the previous record. */
73-
BTR_MODIFY_PREV = 36,
69+
BTR_MODIFY_PREV = 4 | BTR_MODIFY_LEAF,
7470
/** Start searching the entire B-tree. */
75-
BTR_SEARCH_TREE = 37,
71+
BTR_SEARCH_TREE = 8 | BTR_SEARCH_LEAF,
72+
/** Start modifying1 the entire B-tree. */
73+
BTR_MODIFY_TREE = 8 | BTR_MODIFY_LEAF,
7674
/** Continue searching the entire B-tree. */
77-
BTR_CONT_SEARCH_TREE = 38,
75+
BTR_CONT_SEARCH_TREE = 4 | BTR_SEARCH_TREE,
76+
/** Continue modifying the entire B-tree. */
77+
BTR_CONT_MODIFY_TREE = 4 | BTR_MODIFY_TREE,
7878

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

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

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

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

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

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

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

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

153153
/** Try to delete mark the record at the searched position when the
154154
record is in spatial index */
155-
#define BTR_RTREE_DELETE_MARK 524288U
155+
#define BTR_RTREE_DELETE_MARK 32768U
156156

157157
#define BTR_LATCH_MODE_WITHOUT_FLAGS(latch_mode) \
158158
((latch_mode) & ulint(~(BTR_INSERT \

0 commit comments

Comments
 (0)