Skip to content

Conversation

@kamil-holubicki
Copy link
Contributor

https://perconadev.atlassian.net/browse/PS-9647

This patch introduces a new hybrid data structure for MVCC ReadView from Enhanced MySQL

Typically, online transactions are short rather than long, and transaction IDs increase continuously. To leverage these characteristics, a hybrid data structure is used: a static array for consecutive short transaction IDs and a vector for long transactions. With a 2048-byte array, up to 16,384 consecutive active transaction IDs can be stored, each bit representing a transaction ID.

The minimum short transaction ID is used to differentiate between short and long transactions.

IDs smaller than this minimum go into the long transaction vector, while IDs equal to or greater than it are placed in the short transaction array.

For an ID in changes_visible, if it is below the minimum short transaction ID, a direct query is made to the vector, which is efficient due to the generally small number of long transactions.

If the ID is equal to or above the minimum short transaction ID, a bitwise query is performed, with a time complexity of O(1), compared to the previous O(log n) complexity. This improvement enhances efficiency and reduces cache migration between NUMA nodes, as O(1) queries typically complete within a single CPU time slice.

  • In the short_rw_trx_ids_bitmap structure, MAX_SHORT_ACTIVE_BYTES is set to 65536, theoretically accommodating up to 524,288 consecutive short transaction IDs.

  • If the limit is exceeded, the oldest short transaction IDs are converted into long transactions and stored in long_rw_trx_ids.

  • Global long and short transactions are distinguished by min_short_valid_id: IDs smaller than this value are treated as global long transactions, while IDs equal to or greater are considered global short transactions.

During the copying process from the global active transaction list, the short_rw_trx_ids_bitmap structure, which uses only one bit per transaction ID, allows for much higher copying efficiency compared to the native MySQL solution.

For example, with 1000 active transactions, the native MySQL version would require copying at least 8000 bytes, whereas the optimized solution may only need a few hundred bytes. This results in a significant improvement in copying efficiency.

https://perconadev.atlassian.net/browse/PS-9647

This patch introduces a new hybrid data structure for MVCC ReadView from Enhanced MySQL

Typically, online transactions are short rather than long, and transaction IDs
increase continuously. To leverage these characteristics, a hybrid data
structure is used: a static array for consecutive short transaction IDs and a
vector for long transactions. With a 2048-byte array, up to 16,384 consecutive
active transaction IDs can be stored, each bit representing a transaction ID.

The minimum short transaction ID is used to differentiate between short and long
transactions.

IDs smaller than this minimum go into the long transaction vector, while IDs
equal to or greater than it are placed in the short transaction array.

For an ID in changes_visible, if it is below the minimum short transaction ID, a
direct query is made to the vector, which is efficient due to the generally
small number of long transactions.

If the ID is equal to or above the minimum short transaction ID, a bitwise query
is performed, with a time complexity of O(1), compared to the previous O(log n)
complexity. This improvement enhances efficiency and reduces cache migration
between NUMA nodes, as O(1) queries typically complete within a single CPU time
slice.

- In the short_rw_trx_ids_bitmap structure, MAX_SHORT_ACTIVE_BYTES is set to
  65536, theoretically accommodating up to 524,288 consecutive short transaction
  IDs.

- If the limit is exceeded, the oldest short transaction IDs are converted into
  long transactions and stored in long_rw_trx_ids.

- Global long and short transactions are distinguished by min_short_valid_id:
  IDs smaller than this value are treated as global long transactions, while IDs
  equal to or greater are considered global short transactions.

During the copying process from the global active transaction list, the
short_rw_trx_ids_bitmap structure, which uses only one bit per transaction ID,
allows for much higher copying efficiency compared to the native MySQL solution.

For example, with 1000 active transactions, the native MySQL version would
require copying at least 8000 bytes, whereas the optimized solution may only
need a few hundred bytes. This results in a significant improvement in copying
efficiency.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/10)

ut_ad(trx_sys_mutex_own());
static inline void update_short_bitmamp(unsigned char *short_bitmap,
trx_id_t id, bool is_set_value) {
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

trx_id_t id, bool is_set_value) {
unsigned int trim_id = id & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
if (is_set_value) {
short_bitmap[array_index] |= (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

if (is_set_value) {
short_bitmap[array_index] |= (1 << (7 - array_remainder));
} else {
short_bitmap[array_index] &= (255 - (1 << (7 - array_remainder)));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

trx->id =
trx->preallocated_id ? trx->preallocated_id : trx_sys_allocate_trx_id();
static inline trx_id_t find_smallest_short_active_trx_id(
unsigned char *short_bitmap, trx_id_t from, trx_id_t to) {

Choose a reason for hiding this comment

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

⚠️ readability-non-const-parameter ⚠️
pointer parameter short_bitmap can be pointer to const

Suggested change
unsigned char *short_bitmap, trx_id_t from, trx_id_t to) {
const unsigned char *short_bitmap, trx_id_t from, trx_id_t to) {

trx_sys->rw_trx_ids.insert(upper_bound_it, trx->id);
trx_id_t start = from;
do {
unsigned int trim_id = start & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

do {
unsigned int trim_id = start & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int trim_id = start & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (is_value_set) {
if (is_value_set != 0) {

Comment on lines +1319 to +1320
} else {
start++;

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
} else {
start++;
} start++;

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (2/10)

if (start > to) {
return to;
}
}

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
}

trx_sys->max_short_valid_id = trx->id;
}

int64_t diff = trx_sys->max_short_valid_id - trx_sys->min_short_valid_id;

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned long to signed type int64_t (aka long) is implementation-defined

trx_id_t old_id_start = trx_sys->min_short_valid_id;
trx_id_t max_short_valid_id = trx_sys->max_short_valid_id;
trx_id_t max_valid_id = max_short_valid_id;
max_valid_id = max_valid_id - ((max_valid_id & 0x7));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

trx_id_t old_id_end = base - 1;

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (is_value_set) {
if (is_value_set != 0) {

trx_sys->long_rw_trx_ids.push_back(id);
trx_sys->short_rw_trx_valid_number--;
short_trx_id_bitmap[array_index] &=
(255 - (1 << (7 - array_remainder)));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

trx_id_t max_short_valid_id = trx_sys->max_short_valid_id;

if (trx->id < min_short_valid_id) {
trx_ids_t::iterator it =

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable it is not initialized

Suggested change
trx_ids_t::iterator it =
trx_ids_t::iterator it = 0 =

max_short_valid_id);
trx_sys->min_short_valid_id = candidate_min_short_valid_id;
} else {
int64_t diff = max_short_valid_id - min_short_valid_id;

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned long to signed type int64_t (aka long) is implementation-defined

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (3/10)

if (diff >= MAX_SHORT_ACTIVE_BYTES) {
trx_id_t old_id_start = min_short_valid_id;
trx_id_t max_valid_id = max_short_valid_id;
max_valid_id = max_valid_id - ((max_valid_id & 0x7));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

trx_id_t old_id_end = base - 1;

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (is_value_set) {
if (is_value_set != 0) {

trx_sys->long_rw_trx_ids.push_back(id);
trx_sys->short_rw_trx_valid_number--;
short_trx_id_bitmap[array_index] &=
(255 - (1 << (7 - array_remainder)));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

We should remove transactions from the list before committing in memory and
releasing locks to ensure right order of removal and consistent snapshot. */
trx_ids_t rw_trx_ids;
trx_ids_t long_rw_trx_ids;

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable long_rw_trx_ids has public visibility

releasing locks to ensure right order of removal and consistent snapshot. */
trx_ids_t rw_trx_ids;
trx_ids_t long_rw_trx_ids;
unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES];

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable short_rw_trx_ids_bitmap has public visibility

trx_ids_t rw_trx_ids;
trx_ids_t long_rw_trx_ids;
unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES];
int short_rw_trx_valid_number;

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable short_rw_trx_valid_number has public visibility

trx_ids_t long_rw_trx_ids;
unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES];
int short_rw_trx_valid_number;
trx_id_t min_short_valid_id;

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable min_short_valid_id has public visibility

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (4/10)

unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES];
int short_rw_trx_valid_number;
trx_id_t min_short_valid_id;
trx_id_t max_short_valid_id;

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable max_short_valid_id has public visibility

m_up_limit_id(),
m_creator_trx_id(),
m_ids(),
m_long_ids(),

Choose a reason for hiding this comment

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

⚠️ readability-redundant-member-init ⚠️
initializer for member m_long_ids is redundant

Suggested change
m_long_ids(),
,

m_long_ids.resize(size);

ids_t::value_type *p = m_ids.data();
ids_t::value_type *p = m_long_ids.data();

Choose a reason for hiding this comment

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

⚠️ readability-identifier-length ⚠️
variable name p is too short, expected at least 2 characters

* `to` if no active transaction is found.
*/
static inline trx_id_t find_smallest_short_active_trx_id(
unsigned char *short_bitmap, trx_id_t from, trx_id_t to) {

Choose a reason for hiding this comment

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

⚠️ readability-non-const-parameter ⚠️
pointer parameter short_bitmap can be pointer to const

Suggested change
unsigned char *short_bitmap, trx_id_t from, trx_id_t to) {
const unsigned char *short_bitmap, trx_id_t from, trx_id_t to) {


trx_id_t start = from;
do {
unsigned int trim_id = start & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

do {
unsigned int trim_id = start & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int trim_id = start & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (is_value_set) {
if (is_value_set != 0) {

Comment on lines +508 to +509
} else {
start++;

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
} else {
start++;
} start++;

if (start > to) {
return to;
}
}

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
}

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (5/10)

ut_ad(trx_sys_mutex_own());

unsigned char *short_trx_id_bitmap = trx_sys->short_rw_trx_ids_bitmap;
unsigned int start = trx_sys->min_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant


unsigned char *short_trx_id_bitmap = trx_sys->short_rw_trx_ids_bitmap;
unsigned int start = trx_sys->min_short_valid_id & 0x7FFFF;
unsigned int end = trx_sys->max_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

unsigned int array_index_end = (end >> 3);

if (array_index_start <= array_index_end) {
int diff = array_index_end - array_index_start + 1;

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

trx_id_t old_id_start = trx_sys->min_short_valid_id;
trx_id_t max_short_valid_id = trx_sys->max_short_valid_id;
trx_id_t max_valid_id = max_short_valid_id;
max_valid_id = max_valid_id - ((max_valid_id & 0x7));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

trx_id_t old_id_end = base - 1;

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (is_value_set) {
if (is_value_set != 0) {

trx_sys->long_rw_trx_ids.push_back(id);
trx_sys->short_rw_trx_valid_number--;
short_trx_id_bitmap[array_index] &=
(255 - (1 << (7 - array_remainder)));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

}
}

start = candidate_min_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (6/10)

}

start = candidate_min_short_valid_id & 0x7FFFF;
end = max_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant


array_index_start = (start >> 3);
array_index_end = (end >> 3);
diff = array_index_end - array_index_start + 1;

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

}
} else {
int diff = MAX_SHORT_ACTIVE_BYTES - array_index_start;
int total = diff + array_index_end + 1;

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

if (array_index_end > MAX_TOP_ACTIVE_BYTES) {
trx_id_t max_short_valid_id = trx_sys->max_short_valid_id;
trx_id_t max_valid_id = max_short_valid_id;
max_valid_id = max_valid_id - ((max_valid_id & 0x7));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant


trx_id_t old_id_end = base - 1;
for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (is_value_set) {
if (is_value_set != 0) {

trx_sys->long_rw_trx_ids.push_back(id);
trx_sys->short_rw_trx_valid_number--;
short_trx_id_bitmap[array_index] &=
(255 - (1 << (7 - array_remainder)));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

}
}

start = candidate_min_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (7/10)

}

start = candidate_min_short_valid_id & 0x7FFFF;
end = max_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant


array_index_start = (start >> 3);
array_index_end = (end >> 3);
diff = array_index_end - array_index_start + 1;

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

} else {
trx_id_t max_short_valid_id = trx_sys->max_short_valid_id;
trx_id_t max_valid_id = max_short_valid_id;
max_valid_id = max_valid_id - ((max_valid_id & 0x7));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

trx_id_t old_id_end = base - 1;

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

for (trx_id_t id = old_id_start; id <= old_id_end; id++) {
unsigned int trim_id = id & 0x7FFFF;
unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_remainder = trim_id & (0x7);
int is_value_set =
short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (is_value_set) {
if (is_value_set != 0) {

trx_sys->long_rw_trx_ids.push_back(id);
trx_sys->short_rw_trx_valid_number--;
short_trx_id_bitmap[array_index] &=
(255 - (1 << (7 - array_remainder)));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

}
}

start = candidate_min_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

}

start = candidate_min_short_valid_id & 0x7FFFF;
end = max_short_valid_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (8/10)

array_index_end = (end >> 3);

if (array_index_start <= array_index_end) {
diff = array_index_end - array_index_start + 1;

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

::memmove(top_active, &short_trx_id_bitmap[array_index_start], diff);
} else {
diff = MAX_SHORT_ACTIVE_BYTES - array_index_start;
unsigned char *p = top_active;

Choose a reason for hiding this comment

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

⚠️ readability-identifier-length ⚠️
variable name p is too short, expected at least 2 characters

}
}
} else {
unsigned char *p = top_active;

Choose a reason for hiding this comment

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

⚠️ readability-identifier-length ⚠️
variable name p is too short, expected at least 2 characters


if (!trx_sys->rw_trx_ids.empty()) {
copy_trx_ids(trx_sys->rw_trx_ids);
if (trx_sys->short_rw_trx_valid_number) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (trx_sys->short_rw_trx_valid_number) {
if (trx_sys->short_rw_trx_valid_number != 0) {

if (!m_long_ids.empty()) {
m_up_limit_id = m_long_ids.front();
} else {
if (trx_sys->short_rw_trx_valid_number) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (trx_sys->short_rw_trx_valid_number) {
if (trx_sys->short_rw_trx_valid_number != 0) {

if (!other.m_ids.empty()) {
const ids_t::value_type *p = other.m_ids.data();
if (other.m_has_short_actives) {
unsigned int max_trim_id = other.m_short_max_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

const ids_t::value_type *p = other.m_ids.data();
if (other.m_has_short_actives) {
unsigned int max_trim_id = other.m_short_max_id & 0x7FFFF;
unsigned int min_trim_id = other.m_short_min_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

unsigned int max_array_index = (max_trim_id >> 3);
unsigned int min_array_index = (min_trim_id >> 3);
int diff =
(MAX_SHORT_ACTIVE_BYTES + max_array_index - min_array_index + 1) %

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

}

if (!other.m_long_ids.empty()) {
const ids_t::value_type *p = other.m_long_ids.data();

Choose a reason for hiding this comment

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

⚠️ readability-identifier-length ⚠️
variable name p is too short, expected at least 2 characters

if (m_creator_trx_id > 0) {
m_ids.insert(m_creator_trx_id);
if (m_short_min_id <= m_creator_trx_id) {
unsigned int trim_id = m_creator_trx_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (9/10)

m_ids.insert(m_creator_trx_id);
if (m_short_min_id <= m_creator_trx_id) {
unsigned int trim_id = m_creator_trx_id & 0x7FFFF;
unsigned int trim_min_id = m_short_min_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

unsigned int array_min_index = (trim_min_id >> 3);
array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) %
MAX_TOP_ACTIVE_BYTES;
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) %
MAX_TOP_ACTIVE_BYTES;
unsigned int array_remainder = trim_id & (0x7);
top_active[array_index] |= (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

unsigned int array_remainder = trim_id & (0x7);
top_active[array_index] |= (1 << (7 - array_remainder));
} else {
m_long_ids.insert(m_creator_trx_id);

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from trx_id_t (aka unsigned long) to signed type ReadView::ids_t::value_type (aka int) is implementation-defined

/** Read view lists the trx ids of those transactions for which a consistent
read should not see the modifications to the database. */

#define MAX_TOP_ACTIVE_BYTES 8192

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
macro MAX_TOP_ACTIVE_BYTES used to declare a constant; consider using a constexpr constant

read should not see the modifications to the database. */

#define MAX_TOP_ACTIVE_BYTES 8192
#define MAX_SHORT_ACTIVE_BYTES 65536

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
macro MAX_SHORT_ACTIVE_BYTES used to declare a constant; consider using a constexpr constant

return (false);

} else if (m_ids.empty()) {
} else if (empty()) {

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
} else if (empty()) {
} if (empty()) {

if (id > m_short_max_id) {
return false;
}
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

return false;
}
unsigned int trim_id = id & 0x7FFFF;
unsigned int trim_min_id = m_short_min_id & 0x7FFFF;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

unsigned int array_min_index = (trim_min_id >> 3);
array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) %
MAX_TOP_ACTIVE_BYTES;
unsigned int array_remainder = trim_id & (0x7);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (10/10)

array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) %
MAX_TOP_ACTIVE_BYTES;
unsigned int array_remainder = trim_id & (0x7);
int is_value_set = top_active[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

MAX_TOP_ACTIVE_BYTES;
unsigned int array_remainder = trim_id & (0x7);
int is_value_set = top_active[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Comment on lines +196 to +200
if (is_value_set) {
return false;
} else {
return true;
}

Choose a reason for hiding this comment

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

⚠️ readability-simplify-boolean-expr ⚠️
redundant boolean literal in conditional return statement

Suggested change
if (is_value_set) {
return false;
} else {
return true;
}
return is_value_set == 0;

int is_value_set = top_active[array_index] & (1 << (7 - array_remainder));
if (is_value_set) {
return false;
} else {

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

}
}

const ids_t::value_type *p = m_long_ids.data();

Choose a reason for hiding this comment

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

⚠️ readability-identifier-length ⚠️
variable name p is too short, expected at least 2 characters

Comment on lines +263 to +267
if (!m_has_short_actives) {
return true;
} else {
return false;
}

Choose a reason for hiding this comment

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

⚠️ readability-simplify-boolean-expr ⚠️
redundant boolean literal in conditional return statement

Suggested change
if (!m_has_short_actives) {
return true;
} else {
return false;
}
return !m_has_short_actives;

if (long_empty) {
if (!m_has_short_actives) {
return true;
} else {

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant